From c888a7533ca1eea7c5039d14f9faf28aec0b59f8 Mon Sep 17 00:00:00 2001 From: kyle-sammons Date: Fri, 12 Jul 2024 11:23:59 -0700 Subject: [PATCH 1/6] Rework query parsing to use the OpenSearch query parser, enabling us to handle a wider variety of queries --- .../slack/astra/chunk/ReadOnlyChunkImpl.java | 3 +- .../com/slack/astra/chunk/ReadWriteChunk.java | 3 +- .../elasticsearchApi/OpenSearchRequest.java | 8 + .../opensearch/OpenSearchAdapter.java | 43 ++- .../logstore/search/LogIndexSearcher.java | 9 +- .../logstore/search/LogIndexSearcherImpl.java | 7 +- .../astra/logstore/search/SearchQuery.java | 8 +- .../logstore/search/SearchResultUtils.java | 29 +- astra/src/main/proto/astra_search.proto | 3 + .../astra/chunk/IndexingChunkImplTest.java | 27 +- .../astra/chunk/ReadOnlyChunkImplTest.java | 9 +- .../astra/chunk/RecoveryChunkImplTest.java | 27 +- .../IndexingChunkManagerTest.java | 12 +- .../RecoveryChunkManagerTest.java | 12 +- .../OpenSearchRequestTest.java | 68 ++++ .../logstore/LuceneIndexStoreImplTest.java | 9 +- .../opensearch/OpenSearchAdapterTest.java | 25 +- .../schema/FieldConflictStrategyTests.java | 34 +- .../AlreadyClosedLogIndexSearcherImpl.java | 4 +- .../IllegalArgumentLogIndexSearcherImpl.java | 4 +- .../search/LogIndexSearcherImplTest.java | 259 +++++++++------ .../SearchResultAggregatorImplTest.java | 24 +- .../search/SearchResultUtilsTest.java | 302 ++++++++++++++++++ .../logstore/search/StatsCollectorTest.java | 3 +- .../slack/astra/server/AstraIndexerTest.java | 3 +- .../com/slack/astra/testlib/SpanUtil.java | 10 + ...TemporaryLogStoreAndSearcherExtension.java | 13 +- .../writer/LogMessageWriterImplTest.java | 6 +- 28 files changed, 793 insertions(+), 171 deletions(-) diff --git a/astra/src/main/java/com/slack/astra/chunk/ReadOnlyChunkImpl.java b/astra/src/main/java/com/slack/astra/chunk/ReadOnlyChunkImpl.java index 1f2db64f16..49041adeb2 100644 --- a/astra/src/main/java/com/slack/astra/chunk/ReadOnlyChunkImpl.java +++ b/astra/src/main/java/com/slack/astra/chunk/ReadOnlyChunkImpl.java @@ -391,7 +391,8 @@ public SearchResult query(SearchQuery query) { searchStartTime, searchEndTime, query.howMany, - query.aggBuilder); + query.aggBuilder, + query.queryBuilder); } else { return (SearchResult) SearchResult.empty(); } diff --git a/astra/src/main/java/com/slack/astra/chunk/ReadWriteChunk.java b/astra/src/main/java/com/slack/astra/chunk/ReadWriteChunk.java index 9ce96d8a02..fe10992218 100644 --- a/astra/src/main/java/com/slack/astra/chunk/ReadWriteChunk.java +++ b/astra/src/main/java/com/slack/astra/chunk/ReadWriteChunk.java @@ -291,7 +291,8 @@ public SearchResult query(SearchQuery query) { query.startTimeEpochMs, query.endTimeEpochMs, query.howMany, - query.aggBuilder); + query.aggBuilder, + query.queryBuilder); } @Override diff --git a/astra/src/main/java/com/slack/astra/elasticsearchApi/OpenSearchRequest.java b/astra/src/main/java/com/slack/astra/elasticsearchApi/OpenSearchRequest.java index 6bc50aff95..18941a182d 100644 --- a/astra/src/main/java/com/slack/astra/elasticsearchApi/OpenSearchRequest.java +++ b/astra/src/main/java/com/slack/astra/elasticsearchApi/OpenSearchRequest.java @@ -63,11 +63,19 @@ public List parseHttpPostBody(String postBody) .setStartTimeEpochMs(getStartTimeEpochMs(body)) .setEndTimeEpochMs(getEndTimeEpochMs(body)) .setAggregations(getAggregations(body)) + .setQuery(getQuery(body)) .build()); } return searchRequests; } + private static String getQuery(JsonNode body) { + if (!body.get("query").isNull() && !body.get("query").isEmpty()) { + return body.get("query").toString(); + } + return null; + } + private static String getDataset(JsonNode header) { return header.get("index").asText(); } diff --git a/astra/src/main/java/com/slack/astra/logstore/opensearch/OpenSearchAdapter.java b/astra/src/main/java/com/slack/astra/logstore/opensearch/OpenSearchAdapter.java index b1a65a666e..8e5c4c8585 100644 --- a/astra/src/main/java/com/slack/astra/logstore/opensearch/OpenSearchAdapter.java +++ b/astra/src/main/java/com/slack/astra/logstore/opensearch/OpenSearchAdapter.java @@ -3,6 +3,7 @@ import static java.util.Collections.emptyList; import static java.util.Collections.emptyMap; import static java.util.Collections.singletonMap; +import static org.opensearch.common.settings.IndexScopedSettings.BUILT_IN_INDEX_SETTINGS; import com.slack.astra.logstore.LogMessage; import com.slack.astra.logstore.search.aggregations.AggBuilder; @@ -30,8 +31,10 @@ import java.time.ZoneId; import java.util.ArrayList; import java.util.Collection; +import java.util.HashSet; import java.util.List; import java.util.Map; +import java.util.Set; import java.util.TreeMap; import java.util.stream.Collectors; import org.apache.commons.lang3.ObjectUtils; @@ -44,6 +47,8 @@ import org.opensearch.cluster.metadata.IndexMetadata; import org.opensearch.common.CheckedConsumer; import org.opensearch.common.compress.CompressedXContent; +import org.opensearch.common.settings.IndexScopedSettings; +import org.opensearch.common.settings.Setting; import org.opensearch.common.settings.Settings; import org.opensearch.common.util.BigArrays; import org.opensearch.common.xcontent.XContentFactory; @@ -60,6 +65,7 @@ import org.opensearch.index.mapper.MappedFieldType; import org.opensearch.index.mapper.MapperService; import org.opensearch.index.query.BoolQueryBuilder; +import org.opensearch.index.query.QueryBuilder; import org.opensearch.index.query.QueryShardContext; import org.opensearch.index.query.QueryStringQueryBuilder; import org.opensearch.index.query.RangeQueryBuilder; @@ -127,16 +133,22 @@ public class OpenSearchAdapter { // set this to a high number for now private static final int TOTAL_FIELDS_LIMIT = 2500; + // This will enable OpenSearch query parsing by default, rather than going down the + // QueryString parsing path we have been using + private boolean useOpenSearchQueryParsing = false; + public OpenSearchAdapter(Map chunkSchema) { this.indexSettings = buildIndexSettings(); this.similarityService = new SimilarityService(indexSettings, null, emptyMap()); this.mapperService = buildMapperService(indexSettings, similarityService); this.chunkSchema = chunkSchema; + this.useOpenSearchQueryParsing = + Boolean.parseBoolean(System.getProperty("astra.bulkIngest.useKafkaTransactions", "false")); } /** - * Builds a Lucene query using the provided arguments, and the currently loaded schema. Uses the - * Opensearch Query String builder. TODO - use the dataset param in building query + * Builds a Lucene query using the provided arguments, and the currently loaded schema. Uses + * Opensearch QueryBuilder's. TODO - use the dataset param in building query * * @see Query * parsing OpenSearch docs @@ -149,7 +161,8 @@ public Query buildQuery( String queryStr, Long startTimeMsEpoch, Long endTimeMsEpoch, - IndexSearcher indexSearcher) + IndexSearcher indexSearcher, + QueryBuilder queryBuilder) throws IOException { LOG.trace("Query raw input string: '{}'", queryStr); QueryShardContext queryShardContext = @@ -159,6 +172,11 @@ public Query buildQuery( indexSearcher, similarityService, mapperService); + + if (queryBuilder != null && this.useOpenSearchQueryParsing) { + return queryBuilder.rewrite(queryShardContext).toQuery(queryShardContext); + } + try { BoolQueryBuilder boolQueryBuilder = new BoolQueryBuilder(); @@ -192,11 +210,14 @@ public Query buildQuery( if (queryShardContext.getMapperService().fieldType(LogMessage.SystemField.ALL.fieldName) != null) { - queryStringQueryBuilder.defaultField(LogMessage.SystemField.ALL.fieldName); // setting lenient=false will not throw error when the query fails to parse against // numeric fields queryStringQueryBuilder.lenient(false); } else { + // The _all field is the default field for all queries. If we explicitly don't want + // to search that field, or that field isn't mapped, then we need to set the default to be + // * + queryStringQueryBuilder.defaultField("*"); queryStringQueryBuilder.lenient(true); } @@ -393,9 +414,21 @@ protected static IndexSettings buildIndexSettings() { // index sort info to be present as a setting here. .put("index.sort.field", LogMessage.SystemField.TIME_SINCE_EPOCH.fieldName) .put("index.sort.order", "desc") + .put("index.query.default_field", LogMessage.SystemField.ALL.fieldName) + .put("index.query_string.lenient", false) .build(); + + Settings nodeSetings = + Settings.builder().put("indices.query.query_string.analyze_wildcard", true).build(); + + Set> built = new HashSet<>(BUILT_IN_INDEX_SETTINGS); + + IndexScopedSettings indexScopedSettings = new IndexScopedSettings(settings, built); + return new IndexSettings( - IndexMetadata.builder("index").settings(settings).build(), Settings.EMPTY); + IndexMetadata.builder("index").settings(settings).build(), + nodeSetings, + indexScopedSettings); } /** diff --git a/astra/src/main/java/com/slack/astra/logstore/search/LogIndexSearcher.java b/astra/src/main/java/com/slack/astra/logstore/search/LogIndexSearcher.java index bc00ff552f..341b1f68cf 100644 --- a/astra/src/main/java/com/slack/astra/logstore/search/LogIndexSearcher.java +++ b/astra/src/main/java/com/slack/astra/logstore/search/LogIndexSearcher.java @@ -2,8 +2,15 @@ import com.slack.astra.logstore.search.aggregations.AggBuilder; import java.io.Closeable; +import org.opensearch.index.query.QueryBuilder; public interface LogIndexSearcher extends Closeable { SearchResult search( - String dataset, String query, Long minTime, Long maxTime, int howMany, AggBuilder aggBuilder); + String dataset, + String query, + Long minTime, + Long maxTime, + int howMany, + AggBuilder aggBuilder, + QueryBuilder queryBuilder); } diff --git a/astra/src/main/java/com/slack/astra/logstore/search/LogIndexSearcherImpl.java b/astra/src/main/java/com/slack/astra/logstore/search/LogIndexSearcherImpl.java index e8c2b579db..f0cb3f80aa 100644 --- a/astra/src/main/java/com/slack/astra/logstore/search/LogIndexSearcherImpl.java +++ b/astra/src/main/java/com/slack/astra/logstore/search/LogIndexSearcherImpl.java @@ -35,6 +35,7 @@ import org.apache.lucene.search.TopFieldCollector; import org.apache.lucene.search.TopFieldDocs; import org.apache.lucene.store.MMapDirectory; +import org.opensearch.index.query.QueryBuilder; import org.opensearch.search.aggregations.InternalAggregation; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -87,7 +88,8 @@ public SearchResult search( Long startTimeMsEpoch, Long endTimeMsEpoch, int howMany, - AggBuilder aggBuilder) { + AggBuilder aggBuilder, + QueryBuilder queryBuilder) { ensureNonEmptyString(dataset, "dataset should be a non-empty string"); ensureNonNullString(queryStr, "query should be a non-empty string"); @@ -111,12 +113,13 @@ public SearchResult search( // Acquire an index searcher from searcher manager. // This is a useful optimization for indexes that are static. IndexSearcher searcher = searcherManager.acquire(); + try { List results; InternalAggregation internalAggregation = null; Query query = openSearchAdapter.buildQuery( - dataset, queryStr, startTimeMsEpoch, endTimeMsEpoch, searcher); + dataset, queryStr, startTimeMsEpoch, endTimeMsEpoch, searcher, queryBuilder); if (howMany > 0) { CollectorManager topFieldCollector = diff --git a/astra/src/main/java/com/slack/astra/logstore/search/SearchQuery.java b/astra/src/main/java/com/slack/astra/logstore/search/SearchQuery.java index b2de31816f..e002b471fa 100644 --- a/astra/src/main/java/com/slack/astra/logstore/search/SearchQuery.java +++ b/astra/src/main/java/com/slack/astra/logstore/search/SearchQuery.java @@ -2,6 +2,7 @@ import com.slack.astra.logstore.search.aggregations.AggBuilder; import java.util.List; +import org.opensearch.index.query.QueryBuilder; /** A class that represents a search query internally to LogStore. */ public class SearchQuery { @@ -9,6 +10,7 @@ public class SearchQuery { @Deprecated public final String dataset; public final String queryStr; + public final QueryBuilder queryBuilder; public final long startTimeEpochMs; public final long endTimeEpochMs; public final int howMany; @@ -22,7 +24,8 @@ public SearchQuery( long endTimeEpochMs, int howMany, AggBuilder aggBuilder, - List chunkIds) { + List chunkIds, + QueryBuilder queryBuilder) { this.dataset = dataset; this.queryStr = queryStr; this.startTimeEpochMs = startTimeEpochMs; @@ -30,6 +33,7 @@ public SearchQuery( this.howMany = howMany; this.aggBuilder = aggBuilder; this.chunkIds = chunkIds; + this.queryBuilder = queryBuilder; } @Override @@ -51,6 +55,8 @@ public String toString() { + chunkIds + ", aggBuilder=" + aggBuilder + + ", queryBuilder=" + + queryBuilder + '}'; } } diff --git a/astra/src/main/java/com/slack/astra/logstore/search/SearchResultUtils.java b/astra/src/main/java/com/slack/astra/logstore/search/SearchResultUtils.java index 22a2b999be..c59cb3f1c9 100644 --- a/astra/src/main/java/com/slack/astra/logstore/search/SearchResultUtils.java +++ b/astra/src/main/java/com/slack/astra/logstore/search/SearchResultUtils.java @@ -3,6 +3,7 @@ import brave.ScopedSpan; import brave.Tracing; import com.fasterxml.jackson.core.JsonProcessingException; +import com.fasterxml.jackson.databind.ObjectMapper; import com.google.protobuf.ByteString; import com.slack.astra.logstore.LogMessage; import com.slack.astra.logstore.LogWireMessage; @@ -34,6 +35,13 @@ import java.util.Map; import java.util.stream.Collectors; import org.apache.commons.lang3.NotImplementedException; +import org.opensearch.common.settings.Settings; +import org.opensearch.common.xcontent.json.JsonXContentParser; +import org.opensearch.core.xcontent.DeprecationHandler; +import org.opensearch.core.xcontent.NamedXContentRegistry; +import org.opensearch.index.query.AbstractQueryBuilder; +import org.opensearch.index.query.QueryBuilder; +import org.opensearch.search.SearchModule; public class SearchResultUtils { public static Map fromValueStruct(AstraSearch.Struct struct) { @@ -657,6 +665,24 @@ public static AstraSearch.SearchRequest.SearchAggregation.FiltersAggregation toF } public static SearchQuery fromSearchRequest(AstraSearch.SearchRequest searchRequest) { + QueryBuilder queryBuilder = null; + if (!searchRequest.getQuery().isEmpty()) { + SearchModule searchModule = new SearchModule(Settings.EMPTY, List.of()); + try { + ObjectMapper objectMapper = new ObjectMapper(); + NamedXContentRegistry namedXContentRegistry = + new NamedXContentRegistry(searchModule.getNamedXContents()); + JsonXContentParser jsonXContentParser = + new JsonXContentParser( + namedXContentRegistry, + DeprecationHandler.IGNORE_DEPRECATIONS, + objectMapper.createParser(searchRequest.getQuery())); + queryBuilder = AbstractQueryBuilder.parseInnerQueryBuilder(jsonXContentParser); + } catch (Exception e) { + throw new RuntimeException(e); + } + } + return new SearchQuery( searchRequest.getDataset(), searchRequest.getQueryString(), @@ -664,7 +690,8 @@ public static SearchQuery fromSearchRequest(AstraSearch.SearchRequest searchRequ searchRequest.getEndTimeEpochMs(), searchRequest.getHowMany(), fromSearchAggregations(searchRequest.getAggregations()), - searchRequest.getChunkIdsList()); + searchRequest.getChunkIdsList(), + queryBuilder); } public static SearchResult fromSearchResultProtoOrEmpty( diff --git a/astra/src/main/proto/astra_search.proto b/astra/src/main/proto/astra_search.proto index e2e9546d7b..9fa529dd07 100644 --- a/astra/src/main/proto/astra_search.proto +++ b/astra/src/main/proto/astra_search.proto @@ -20,6 +20,9 @@ message SearchRequest { // Only a single top-level aggregation is currently supported SearchAggregation aggregations = 7; + // The fully query object, to be used in the leaf nodes and parsed by OpenSearch + string query = 8; + message SearchAggregation { // The type of aggregation (ie, avg, date_histogram, etc) string type = 1; diff --git a/astra/src/test/java/com/slack/astra/chunk/IndexingChunkImplTest.java b/astra/src/test/java/com/slack/astra/chunk/IndexingChunkImplTest.java index 46b7f07ebc..0d7abda8fc 100644 --- a/astra/src/test/java/com/slack/astra/chunk/IndexingChunkImplTest.java +++ b/astra/src/test/java/com/slack/astra/chunk/IndexingChunkImplTest.java @@ -168,7 +168,8 @@ public void testAddAndSearchChunk() { 10, new DateHistogramAggBuilder( "1", LogMessage.SystemField.TIME_SINCE_EPOCH.fieldName, "1s"), - Collections.emptyList())); + Collections.emptyList(), + null)); chunk.query( new SearchQuery( @@ -179,7 +180,8 @@ public void testAddAndSearchChunk() { 10, new DateHistogramAggBuilder( "1", LogMessage.SystemField.TIME_SINCE_EPOCH.fieldName, "1s"), - Collections.emptyList())); + Collections.emptyList(), + null)); SearchResult results = chunk.query( @@ -191,7 +193,8 @@ public void testAddAndSearchChunk() { 10, new DateHistogramAggBuilder( "1", LogMessage.SystemField.TIME_SINCE_EPOCH.fieldName, "1s"), - Collections.emptyList())); + Collections.emptyList(), + null)); assertThat(results.hits.size()).isEqualTo(10); assertThat(getCount(MESSAGES_RECEIVED_COUNTER, registry)).isEqualTo(100); @@ -311,7 +314,8 @@ private void searchChunk( 10, new DateHistogramAggBuilder( "1", LogMessage.SystemField.TIME_SINCE_EPOCH.fieldName, "1s"), - Collections.emptyList())) + Collections.emptyList(), + null)) .hits .size()) .isEqualTo(expectedResultCount); @@ -342,7 +346,8 @@ public void testSearchInReadOnlyChunk() { 10, new DateHistogramAggBuilder( "1", LogMessage.SystemField.TIME_SINCE_EPOCH.fieldName, "1s"), - Collections.emptyList())); + Collections.emptyList(), + null)); assertThat(results.hits.size()).isEqualTo(1); assertThat(getCount(MESSAGES_RECEIVED_COUNTER, registry)).isEqualTo(100); @@ -412,7 +417,8 @@ public void testCommitBeforeSnapshot() { 10, new DateHistogramAggBuilder( "1", LogMessage.SystemField.TIME_SINCE_EPOCH.fieldName, "1s"), - Collections.emptyList())); + Collections.emptyList(), + null)); assertThat(resultsBeforeCommit.hits.size()).isEqualTo(0); // Snapshot forces commit and refresh @@ -428,7 +434,8 @@ public void testCommitBeforeSnapshot() { 10, new DateHistogramAggBuilder( "1", LogMessage.SystemField.TIME_SINCE_EPOCH.fieldName, "1s"), - Collections.emptyList())); + Collections.emptyList(), + null)); assertThat(resultsAfterPreSnapshot.hits.size()).isEqualTo(1); } } @@ -609,7 +616,8 @@ public void testSnapshotToNonExistentS3BucketFails() 10, new DateHistogramAggBuilder( "1", LogMessage.SystemField.TIME_SINCE_EPOCH.fieldName, "1s"), - Collections.emptyList()); + Collections.emptyList(), + null); assertThat(chunk.isReadOnly()).isTrue(); SearchResult resultsAfterPreSnapshot = chunk.query(searchQuery); assertThat(resultsAfterPreSnapshot.hits.size()).isEqualTo(1); @@ -672,7 +680,8 @@ public void testSnapshotToS3UsingChunkApi() throws Exception { 10, new DateHistogramAggBuilder( "1", LogMessage.SystemField.TIME_SINCE_EPOCH.fieldName, "1s"), - Collections.emptyList()); + Collections.emptyList(), + null); assertThat(chunk.isReadOnly()).isTrue(); SearchResult resultsAfterPreSnapshot = chunk.query(searchQuery); assertThat(resultsAfterPreSnapshot.hits.size()).isEqualTo(1); diff --git a/astra/src/test/java/com/slack/astra/chunk/ReadOnlyChunkImplTest.java b/astra/src/test/java/com/slack/astra/chunk/ReadOnlyChunkImplTest.java index 3b14898b47..8bbd2d9082 100644 --- a/astra/src/test/java/com/slack/astra/chunk/ReadOnlyChunkImplTest.java +++ b/astra/src/test/java/com/slack/astra/chunk/ReadOnlyChunkImplTest.java @@ -163,7 +163,8 @@ public void shouldHandleChunkLivecycle() throws Exception { 500, new DateHistogramAggBuilder( "1", LogMessage.SystemField.TIME_SINCE_EPOCH.fieldName, "1s"), - Collections.emptyList())); + Collections.emptyList(), + null)); assertThat(logMessageSearchResult.hits.size()).isEqualTo(10); await() @@ -214,7 +215,8 @@ public void shouldHandleChunkLivecycle() throws Exception { 500, new DateHistogramAggBuilder( "1", LogMessage.SystemField.TIME_SINCE_EPOCH.fieldName, "1s"), - Collections.emptyList())); + Collections.emptyList(), + null)); assertThat(logMessageEmptySearchResult).isEqualTo(SearchResult.empty()); assertThat(readOnlyChunk.info()).isNull(); @@ -427,7 +429,8 @@ public void closeShouldCleanupLiveChunkCorrectly() throws Exception { 500, new DateHistogramAggBuilder( "1", LogMessage.SystemField.TIME_SINCE_EPOCH.fieldName, "1s"), - Collections.emptyList()); + Collections.emptyList(), + null); SearchResult logMessageSearchResult = readOnlyChunk.query(query); assertThat(logMessageSearchResult.hits.size()).isEqualTo(10); assertThat(meterRegistry.get(CHUNK_ASSIGNMENT_TIMER).tag("successful", "true").timer().count()) diff --git a/astra/src/test/java/com/slack/astra/chunk/RecoveryChunkImplTest.java b/astra/src/test/java/com/slack/astra/chunk/RecoveryChunkImplTest.java index faa2fba87b..e7eb3c219e 100644 --- a/astra/src/test/java/com/slack/astra/chunk/RecoveryChunkImplTest.java +++ b/astra/src/test/java/com/slack/astra/chunk/RecoveryChunkImplTest.java @@ -152,7 +152,8 @@ public void testAddAndSearchChunk() { 10, new DateHistogramAggBuilder( "1", LogMessage.SystemField.TIME_SINCE_EPOCH.fieldName, "1s"), - Collections.emptyList())); + Collections.emptyList(), + null)); chunk.query( new SearchQuery( @@ -163,7 +164,8 @@ public void testAddAndSearchChunk() { 10, new DateHistogramAggBuilder( "1", LogMessage.SystemField.TIME_SINCE_EPOCH.fieldName, "1s"), - Collections.emptyList())); + Collections.emptyList(), + null)); SearchResult results = chunk.query( @@ -175,7 +177,8 @@ public void testAddAndSearchChunk() { 10, new DateHistogramAggBuilder( "1", LogMessage.SystemField.TIME_SINCE_EPOCH.fieldName, "1s"), - Collections.emptyList())); + Collections.emptyList(), + null)); assertThat(results.hits.size()).isEqualTo(10); assertThat(getCount(MESSAGES_RECEIVED_COUNTER, registry)).isEqualTo(100); @@ -300,7 +303,8 @@ private void searchChunk( 10, new DateHistogramAggBuilder( "1", LogMessage.SystemField.TIME_SINCE_EPOCH.fieldName, "1s"), - Collections.emptyList())) + Collections.emptyList(), + null)) .hits .size()) .isEqualTo(expectedResultCount); @@ -331,7 +335,8 @@ public void testSearchInReadOnlyChunk() { 10, new DateHistogramAggBuilder( "1", LogMessage.SystemField.TIME_SINCE_EPOCH.fieldName, "1s"), - Collections.emptyList())); + Collections.emptyList(), + null)); assertThat(results.hits.size()).isEqualTo(1); assertThat(getCount(MESSAGES_RECEIVED_COUNTER, registry)).isEqualTo(100); @@ -401,7 +406,8 @@ public void testCommitBeforeSnapshot() { 10, new DateHistogramAggBuilder( "1", LogMessage.SystemField.TIME_SINCE_EPOCH.fieldName, "1s"), - Collections.emptyList())); + Collections.emptyList(), + null)); assertThat(resultsBeforeCommit.hits.size()).isEqualTo(0); // Snapshot forces commit and refresh @@ -417,7 +423,8 @@ public void testCommitBeforeSnapshot() { 10, new DateHistogramAggBuilder( "1", LogMessage.SystemField.TIME_SINCE_EPOCH.fieldName, "1s"), - Collections.emptyList())); + Collections.emptyList(), + null)); assertThat(resultsAfterPreSnapshot.hits.size()).isEqualTo(1); } } @@ -596,7 +603,8 @@ public void testSnapshotToNonExistentS3BucketFails() { 10, new DateHistogramAggBuilder( "1", LogMessage.SystemField.TIME_SINCE_EPOCH.fieldName, "1s"), - Collections.emptyList()); + Collections.emptyList(), + null); assertThat(chunk.isReadOnly()).isTrue(); SearchResult resultsAfterPreSnapshot = chunk.query(searchQuery); assertThat(resultsAfterPreSnapshot.hits.size()).isEqualTo(1); @@ -647,7 +655,8 @@ public void testSnapshotToS3UsingChunkApi() throws Exception { 10, new DateHistogramAggBuilder( "1", LogMessage.SystemField.TIME_SINCE_EPOCH.fieldName, "1s"), - Collections.emptyList()); + Collections.emptyList(), + null); assertThat(chunk.isReadOnly()).isTrue(); SearchResult resultsAfterPreSnapshot = chunk.query(searchQuery); assertThat(resultsAfterPreSnapshot.hits.size()).isEqualTo(1); diff --git a/astra/src/test/java/com/slack/astra/chunkManager/IndexingChunkManagerTest.java b/astra/src/test/java/com/slack/astra/chunkManager/IndexingChunkManagerTest.java index 54713a318c..7a323f2951 100644 --- a/astra/src/test/java/com/slack/astra/chunkManager/IndexingChunkManagerTest.java +++ b/astra/src/test/java/com/slack/astra/chunkManager/IndexingChunkManagerTest.java @@ -415,7 +415,8 @@ public void testAddMessage() throws Exception { 10, new DateHistogramAggBuilder( "1", LogMessage.SystemField.TIME_SINCE_EPOCH.fieldName, "1s"), - Collections.emptyList()); + Collections.emptyList(), + null); SearchResult results = chunkManager.query(searchQuery, Duration.ofMillis(3000)); assertThat(results.hits.size()).isEqualTo(1); @@ -476,7 +477,8 @@ public void testAddMessage() throws Exception { 10, new DateHistogramAggBuilder( "1", LogMessage.SystemField.TIME_SINCE_EPOCH.fieldName, "1s"), - Collections.emptyList()), + Collections.emptyList(), + null), Duration.ofMillis(3000)) .hits .size()) @@ -506,7 +508,8 @@ public void testAddMessage() throws Exception { 10, new DateHistogramAggBuilder( "1", LogMessage.SystemField.TIME_SINCE_EPOCH.fieldName, "1s"), - Collections.emptyList()), + Collections.emptyList(), + null), Duration.ofMillis(3000)) .hits .size()) @@ -600,7 +603,8 @@ private int searchAndGetHitCount( 10, new DateHistogramAggBuilder( "1", LogMessage.SystemField.TIME_SINCE_EPOCH.fieldName, "1s"), - Collections.emptyList()); + Collections.emptyList(), + null); return chunkManager.query(searchQuery, Duration.ofMillis(3000)).hits.size(); } diff --git a/astra/src/test/java/com/slack/astra/chunkManager/RecoveryChunkManagerTest.java b/astra/src/test/java/com/slack/astra/chunkManager/RecoveryChunkManagerTest.java index d05cf1b1df..ed9388c163 100644 --- a/astra/src/test/java/com/slack/astra/chunkManager/RecoveryChunkManagerTest.java +++ b/astra/src/test/java/com/slack/astra/chunkManager/RecoveryChunkManagerTest.java @@ -197,7 +197,8 @@ public void testAddMessageAndRollover() throws Exception { 10, new DateHistogramAggBuilder( "1", LogMessage.SystemField.TIME_SINCE_EPOCH.fieldName, "1s"), - Collections.emptyList()); + Collections.emptyList(), + null); SearchResult results = chunkManager.getActiveChunk().query(searchQuery); assertThat(results.hits.size()).isEqualTo(1); @@ -239,7 +240,8 @@ public void testAddMessageAndRollover() throws Exception { 10, new DateHistogramAggBuilder( "1", LogMessage.SystemField.TIME_SINCE_EPOCH.fieldName, "1s"), - Collections.emptyList())) + Collections.emptyList(), + null)) .hits .size()) .isEqualTo(1); @@ -269,7 +271,8 @@ public void testAddMessageAndRollover() throws Exception { 10, new DateHistogramAggBuilder( "1", LogMessage.SystemField.TIME_SINCE_EPOCH.fieldName, "1s"), - Collections.emptyList())) + Collections.emptyList(), + null)) .hits .size()) .isEqualTo(1); @@ -341,7 +344,8 @@ private void testChunkManagerSearch( 10, new DateHistogramAggBuilder( "1", LogMessage.SystemField.TIME_SINCE_EPOCH.fieldName, "1s"), - Collections.emptyList()); + Collections.emptyList(), + null); SearchResult result = chunkManager.query(searchQuery, Duration.ofMillis(3000)); assertThat(result.hits.size()).isEqualTo(expectedHitCount); diff --git a/astra/src/test/java/com/slack/astra/elasticsearchApi/OpenSearchRequestTest.java b/astra/src/test/java/com/slack/astra/elasticsearchApi/OpenSearchRequestTest.java index 9a1c3dd047..ab55b03ecc 100644 --- a/astra/src/test/java/com/slack/astra/elasticsearchApi/OpenSearchRequestTest.java +++ b/astra/src/test/java/com/slack/astra/elasticsearchApi/OpenSearchRequestTest.java @@ -2,6 +2,8 @@ import static org.assertj.core.api.Assertions.assertThat; +import com.fasterxml.jackson.databind.JsonNode; +import com.fasterxml.jackson.databind.ObjectMapper; import com.google.common.io.Resources; import com.slack.astra.logstore.LogMessage; import com.slack.astra.proto.service.AstraSearch; @@ -9,10 +11,18 @@ import java.nio.charset.Charset; import java.util.List; import java.util.Map; +import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.api.Test; public class OpenSearchRequestTest { + private static ObjectMapper objectMapper; + + @BeforeAll + public static void beforeClass() { + objectMapper = new ObjectMapper(); + } + private String getRawQueryString(String filename) throws IOException { return Resources.toString( Resources.getResource(String.format("opensearchRequest/%s.ndjson", filename)), @@ -36,6 +46,9 @@ public void testNoAggs() throws Exception { assertThat(request.getQueryString()).isEqualTo("*:*"); assertThat(request.getStartTimeEpochMs()).isEqualTo(1680551083859L); assertThat(request.getEndTimeEpochMs()).isEqualTo(1680554683859L); + + JsonNode parsedRequest = objectMapper.readTree(rawRequest.split("\n")[1]); + assertThat(request.getQuery()).isEqualTo(parsedRequest.get("query").toString()); } @Test @@ -55,6 +68,9 @@ public void testGeneralFields() throws Exception { assertThat(request.getQueryString()).isEqualTo("*:*"); assertThat(request.getStartTimeEpochMs()).isEqualTo(1676498801027L); assertThat(request.getEndTimeEpochMs()).isEqualTo(1676500240688L); + + JsonNode parsedRequest = objectMapper.readTree(rawRequest.split("\n")[1]); + assertThat(request.getQuery()).isEqualTo(parsedRequest.get("query").toString()); } @Test @@ -86,6 +102,10 @@ public void testDateHistogram() throws Exception { "max", 1676500240688L)); assertThat(dateHistogramAggregation.getFormat()).isEqualTo("epoch_millis"); assertThat(dateHistogramAggregation.getOffset()).isEqualTo("5s"); + + JsonNode parsedRequest = objectMapper.readTree(rawRequest.split("\n")[1]); + assertThat(parsedRequestList.getFirst().getQuery()) + .isEqualTo(parsedRequest.get("query").toString()); } @Test @@ -110,6 +130,10 @@ public void testHistogram() throws Exception { histogramAggregation = histogramAggBuilder.getValueSource().getHistogram(); assertThat(histogramAggregation.getInterval()).isEqualTo("1000"); assertThat(histogramAggregation.getMinDocCount()).isEqualTo(1); + + JsonNode parsedRequest = objectMapper.readTree(rawRequest.split("\n")[1]); + assertThat(parsedRequestList.getFirst().getQuery()) + .isEqualTo(parsedRequest.get("query").toString()); } @Test @@ -140,6 +164,10 @@ public void testUniqueCount() throws Exception { .getPrecisionThreshold() .getLongValue()) .isEqualTo(1); + + JsonNode parsedRequest = objectMapper.readTree(rawRequest.split("\n")[1]); + assertThat(parsedRequestList.getFirst().getQuery()) + .isEqualTo(parsedRequest.get("query").toString()); } @Test @@ -166,6 +194,10 @@ public void testPercentiles() throws Exception { .containsExactly(25D, 50D, 75D, 95D, 99D); assertThat(percentileAggBuilder.getValueSource().getScript().getStringValue()) .isEqualTo("return 8;"); + + JsonNode parsedRequest = objectMapper.readTree(rawRequest.split("\n")[1]); + assertThat(parsedRequestList.getFirst().getQuery()) + .isEqualTo(parsedRequest.get("query").toString()); } @Test @@ -191,6 +223,10 @@ public void testHistogramWithNestedExtendedStats() throws Exception { assertThat(extendedStatsAgg.getValueSource().getField()).isEqualTo("duration_ms"); assertThat(extendedStatsAgg.getValueSource().getExtendedStats().getSigma().getDoubleValue()) .isEqualTo(2D); + + JsonNode parsedRequest = objectMapper.readTree(rawRequest.split("\n")[1]); + assertThat(parsedRequestList.getFirst().getQuery()) + .isEqualTo(parsedRequest.get("query").toString()); } @Test @@ -217,6 +253,10 @@ public void testHistogramWithNestedCumulativeSum() throws Exception { assertThat( cumulativeSumAggBuilder.getPipeline().getCumulativeSum().getFormat().getStringValue()) .isEqualTo("##0.#####E0"); + + JsonNode parsedRequest = objectMapper.readTree(rawRequest.split("\n")[1]); + assertThat(parsedRequestList.getFirst().getQuery()) + .isEqualTo(parsedRequest.get("query").toString()); } @Test @@ -244,6 +284,10 @@ public void testHistogramWithNestedMovingFunction() throws Exception { .isEqualTo("return 8;"); assertThat(movingFunctionAggBuilder.getPipeline().getMovingFunction().getWindow()).isEqualTo(2); assertThat(movingFunctionAggBuilder.getPipeline().getMovingFunction().getShift()).isEqualTo(3); + + JsonNode parsedRequest = objectMapper.readTree(rawRequest.split("\n")[1]); + assertThat(parsedRequestList.getFirst().getQuery()) + .isEqualTo(parsedRequest.get("query").toString()); } @Test @@ -269,6 +313,10 @@ public void testHistogramWithNestedDerivative() throws Exception { assertThat(derivativeAggBuilder.getPipeline().getBucketsPath()).isEqualTo("_count"); assertThat(derivativeAggBuilder.getPipeline().getDerivative().getUnit().getStringValue()) .isEqualTo("1m"); + + JsonNode parsedRequest = objectMapper.readTree(rawRequest.split("\n")[1]); + assertThat(parsedRequestList.getFirst().getQuery()) + .isEqualTo(parsedRequest.get("query").toString()); } @Test @@ -303,6 +351,10 @@ public void testHistogramWithNestedAvg() throws Exception { parsedRequestList.get(0).getAggregations().getSubAggregations(0); assertThat(avgAggBuilder.getName()).isEqualTo("3"); assertThat(avgAggBuilder.getValueSource().getField()).isEqualTo("duration_ms"); + + JsonNode parsedRequest = objectMapper.readTree(rawRequest.split("\n")[1]); + assertThat(parsedRequestList.getFirst().getQuery()) + .isEqualTo(parsedRequest.get("query").toString()); } @Test @@ -327,6 +379,10 @@ public void testHistogramWithNestedSum() throws Exception { assertThat(sumAggBuilder.getValueSource().getField()).isEqualTo("duration_ms"); assertThat(sumAggBuilder.getValueSource().getScript().getStringValue()).isEqualTo("return 8;"); assertThat(sumAggBuilder.getValueSource().getMissing().getStringValue()).isEqualTo("2"); + + JsonNode parsedRequest = objectMapper.readTree(rawRequest.split("\n")[1]); + assertThat(parsedRequestList.getFirst().getQuery()) + .isEqualTo(parsedRequest.get("query").toString()); } @Test @@ -352,6 +408,10 @@ public void testDateHistogramWithNestedMovingAvg() throws IOException { assertThat(movAvgAggBuilder.getName()).isEqualTo("3"); assertThat(movAvgAggBuilder.getType()).isEqualTo("moving_avg"); assertThat(movAvgAggBuilder.getPipeline().getBucketsPath()).isEqualTo("_count"); + + JsonNode parsedRequest = objectMapper.readTree(rawRequest.split("\n")[1]); + assertThat(parsedRequestList.getFirst().getQuery()) + .isEqualTo(parsedRequest.get("query").toString()); } @Test @@ -378,6 +438,10 @@ public void testFilters() throws IOException { .isEqualTo("*"); assertThat(filtersAggregation.getFilters().getFiltersMap().get("bar").getAnalyzeWildcard()) .isEqualTo(false); + + JsonNode parsedRequest = objectMapper.readTree(rawRequest.split("\n")[1]); + assertThat(parsedRequestList.getFirst().getQuery()) + .isEqualTo(parsedRequest.get("query").toString()); } @Test @@ -399,5 +463,9 @@ public void testTermsAggregation() throws IOException { assertThat(typedTermsAggregation.getMinDocCount()).isEqualTo(1); assertThat(typedTermsAggregation.getSize()).isEqualTo(10); assertThat(typedTermsAggregation.getOrderMap().get("_term")).isEqualTo("desc"); + + JsonNode parsedRequest = objectMapper.readTree(rawRequest.split("\n")[1]); + assertThat(parsedRequestList.getFirst().getQuery()) + .isEqualTo(parsedRequest.get("query").toString()); } } diff --git a/astra/src/test/java/com/slack/astra/logstore/LuceneIndexStoreImplTest.java b/astra/src/test/java/com/slack/astra/logstore/LuceneIndexStoreImplTest.java index 745d4bf575..196a389889 100644 --- a/astra/src/test/java/com/slack/astra/logstore/LuceneIndexStoreImplTest.java +++ b/astra/src/test/java/com/slack/astra/logstore/LuceneIndexStoreImplTest.java @@ -123,7 +123,8 @@ public void testSearchAndQueryDocsWithNestedJson() { MAX_TIME, 100, new DateHistogramAggBuilder( - "1", LogMessage.SystemField.TIME_SINCE_EPOCH.fieldName, "1s")); + "1", LogMessage.SystemField.TIME_SINCE_EPOCH.fieldName, "1s"), + null); assertThat(result1.hits.size()).isEqualTo(1); SearchResult result2 = @@ -134,7 +135,8 @@ public void testSearchAndQueryDocsWithNestedJson() { MAX_TIME, 100, new DateHistogramAggBuilder( - "1", LogMessage.SystemField.TIME_SINCE_EPOCH.fieldName, "1s")); + "1", LogMessage.SystemField.TIME_SINCE_EPOCH.fieldName, "1s"), + null); assertThat(result2.hits.size()).isEqualTo(1); SearchResult result3 = @@ -145,7 +147,8 @@ public void testSearchAndQueryDocsWithNestedJson() { MAX_TIME, 100, new DateHistogramAggBuilder( - "1", LogMessage.SystemField.TIME_SINCE_EPOCH.fieldName, "1s")); + "1", LogMessage.SystemField.TIME_SINCE_EPOCH.fieldName, "1s"), + null); assertThat(result3.hits.size()).isEqualTo(1); } diff --git a/astra/src/test/java/com/slack/astra/logstore/opensearch/OpenSearchAdapterTest.java b/astra/src/test/java/com/slack/astra/logstore/opensearch/OpenSearchAdapterTest.java index 4302b508eb..1b6d6fae77 100644 --- a/astra/src/test/java/com/slack/astra/logstore/opensearch/OpenSearchAdapterTest.java +++ b/astra/src/test/java/com/slack/astra/logstore/opensearch/OpenSearchAdapterTest.java @@ -37,6 +37,8 @@ import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.RegisterExtension; import org.opensearch.index.mapper.Uid; +import org.opensearch.index.query.BoolQueryBuilder; +import org.opensearch.index.query.RangeQueryBuilder; import org.opensearch.search.aggregations.AbstractAggregationBuilder; import org.opensearch.search.aggregations.Aggregator; import org.opensearch.search.aggregations.InternalAggregation; @@ -452,13 +454,25 @@ public void handlesDateHistogramExtendedBoundsMinDocEdgeCases() throws IOExcepti .isThrownBy(() -> collectorManager.newCollector()); } + @Test + public void shouldProduceQueryFromQueryBuilder() throws Exception { + BoolQueryBuilder boolQueryBuilder = + new BoolQueryBuilder().filter(new RangeQueryBuilder("_timesinceepoch").gte(1).lte(100)); + IndexSearcher indexSearcher = logStoreAndSearcherRule.logStore.getSearcherManager().acquire(); + Query rangeQuery = + openSearchAdapter.buildQuery("foo", null, null, null, indexSearcher, boolQueryBuilder); + assertThat(rangeQuery).isNotNull(); + assertThat(rangeQuery.toString()).isEqualTo("#_timesinceepoch:[1 TO 100]"); + } + @Test public void shouldParseIdFieldSearch() throws Exception { String idField = "_id"; String idValue = "1"; IndexSearcher indexSearcher = logStoreAndSearcherRule.logStore.getSearcherManager().acquire(); Query idQuery = - openSearchAdapter.buildQuery("foo", STR."\{idField}:\{idValue}", null, null, indexSearcher); + openSearchAdapter.buildQuery( + "foo", STR."\{idField}:\{idValue}", null, null, indexSearcher, null); BytesRef queryStrBytes = new BytesRef(Uid.encodeId("1").bytes); // idQuery.toString="#_id:([fe 1f])" // queryStrBytes.toString="[fe 1f]" @@ -468,11 +482,13 @@ public void shouldParseIdFieldSearch() throws Exception { @Test public void shouldExcludeDateFilterWhenNullTimestamps() throws Exception { IndexSearcher indexSearcher = logStoreAndSearcherRule.logStore.getSearcherManager().acquire(); - Query nullBothTimestamps = openSearchAdapter.buildQuery("foo", "", null, null, indexSearcher); + Query nullBothTimestamps = + openSearchAdapter.buildQuery("foo", "", null, null, indexSearcher, null); // null for both timestamps with no query string should be optimized into a matchall assertThat(nullBothTimestamps).isInstanceOf(MatchAllDocsQuery.class); - Query nullStartTimestamp = openSearchAdapter.buildQuery("foo", "a", null, 100L, indexSearcher); + Query nullStartTimestamp = + openSearchAdapter.buildQuery("foo", "a", null, 100L, indexSearcher, null); assertThat(nullStartTimestamp).isInstanceOf(BooleanQuery.class); Optional filterNullStartQuery = @@ -492,7 +508,8 @@ public void shouldExcludeDateFilterWhenNullTimestamps() throws Exception { assertThat(filterNullStartQuery.get().toString()).contains(String.valueOf(Long.MIN_VALUE)); assertThat(filterNullStartQuery.get().toString()).contains(String.valueOf(100L)); - Query nullEndTimestamp = openSearchAdapter.buildQuery("foo", "", 100L, null, indexSearcher); + Query nullEndTimestamp = + openSearchAdapter.buildQuery("foo", "", 100L, null, indexSearcher, null); Optional filterNullEndQuery = ((BooleanQuery) nullEndTimestamp) .clauses().stream() diff --git a/astra/src/test/java/com/slack/astra/logstore/schema/FieldConflictStrategyTests.java b/astra/src/test/java/com/slack/astra/logstore/schema/FieldConflictStrategyTests.java index a43384b905..111736fc48 100644 --- a/astra/src/test/java/com/slack/astra/logstore/schema/FieldConflictStrategyTests.java +++ b/astra/src/test/java/com/slack/astra/logstore/schema/FieldConflictStrategyTests.java @@ -84,7 +84,7 @@ public void testFieldConflictNumericValues() throws JsonProcessingException { 2, "Test message", Instant.now(), List.of(hostField, tagField, conflictingTagInt)); Document msg1Doc = raiseErrorDocBuilder.fromMessage(doc1); - assertThat(msg1Doc.getFields().size()).isEqualTo(28); + assertThat(msg1Doc.getFields().size()).isEqualTo(30); try { raiseErrorDocBuilder.fromMessage(doc2); @@ -94,23 +94,23 @@ public void testFieldConflictNumericValues() throws JsonProcessingException { } msg1Doc = dropFieldDocBuilder.fromMessage(doc1); - assertThat(msg1Doc.getFields().size()).isEqualTo(28); + assertThat(msg1Doc.getFields().size()).isEqualTo(30); Document msg2Doc = dropFieldDocBuilder.fromMessage(doc2); // 2 less because docValue is also missing - assertThat(msg2Doc.getFields().size()).isEqualTo(26); + assertThat(msg2Doc.getFields().size()).isEqualTo(28); msg1Doc = convertFieldDocBuilder.fromMessage(doc1); - assertThat(msg1Doc.getFields().size()).isEqualTo(28); + assertThat(msg1Doc.getFields().size()).isEqualTo(30); msg2Doc = convertFieldDocBuilder.fromMessage(doc2); - assertThat(msg2Doc.getFields().size()).isEqualTo(28); + assertThat(msg2Doc.getFields().size()).isEqualTo(30); msg1Doc = convertAndDuplicateFieldDocBuilder.fromMessage(doc1); - assertThat(msg1Doc.getFields().size()).isEqualTo(28); + assertThat(msg1Doc.getFields().size()).isEqualTo(30); msg2Doc = convertAndDuplicateFieldDocBuilder.fromMessage(doc2); - assertThat(msg2Doc.getFields().size()).isEqualTo(30); + assertThat(msg2Doc.getFields().size()).isEqualTo(32); String additionalCreatedFieldName = makeNewFieldOfType(conflictingFieldName, FieldType.INTEGER); // Value converted and new field is added. assertThat(getFieldCount(msg2Doc, Set.of(conflictingFieldName, additionalCreatedFieldName))) @@ -170,7 +170,7 @@ public void testFieldConflictBooleanValues() throws JsonProcessingException { 3, "Test message", Instant.now(), List.of(hostField, tagField, conflictingTagInt)); Document msg1Doc = raiseErrorDocBuilder.fromMessage(doc1); - assertThat(msg1Doc.getFields().size()).isEqualTo(28); + assertThat(msg1Doc.getFields().size()).isEqualTo(30); try { raiseErrorDocBuilder.fromMessage(doc2); @@ -187,40 +187,40 @@ public void testFieldConflictBooleanValues() throws JsonProcessingException { } msg1Doc = dropFieldDocBuilder.fromMessage(doc1); - assertThat(msg1Doc.getFields().size()).isEqualTo(28); + assertThat(msg1Doc.getFields().size()).isEqualTo(30); Document msg2Doc = dropFieldDocBuilder.fromMessage(doc2); // 2 less because docValue is also missing - assertThat(msg2Doc.getFields().size()).isEqualTo(26); + assertThat(msg2Doc.getFields().size()).isEqualTo(28); Document msg3Doc = dropFieldDocBuilder.fromMessage(doc3); // 2 less because docValue is also missing - assertThat(msg3Doc.getFields().size()).isEqualTo(26); + assertThat(msg3Doc.getFields().size()).isEqualTo(28); msg1Doc = convertFieldDocBuilder.fromMessage(doc1); - assertThat(msg1Doc.getFields().size()).isEqualTo(28); + assertThat(msg1Doc.getFields().size()).isEqualTo(30); msg2Doc = convertFieldDocBuilder.fromMessage(doc2); - assertThat(msg2Doc.getFields().size()).isEqualTo(28); + assertThat(msg2Doc.getFields().size()).isEqualTo(30); // If this ever fails is because message ordering changed the getField returns the DV variant assertThat(msg2Doc.getField(conflictingFieldName).binaryValue().utf8ToString()).isEqualTo("F"); msg3Doc = convertFieldDocBuilder.fromMessage(doc3); - assertThat(msg3Doc.getFields().size()).isEqualTo(28); + assertThat(msg3Doc.getFields().size()).isEqualTo(30); assertThat(msg3Doc.getField(conflictingFieldName).binaryValue().utf8ToString()).isEqualTo("T"); msg1Doc = convertAndDuplicateFieldDocBuilder.fromMessage(doc1); - assertThat(msg1Doc.getFields().size()).isEqualTo(28); + assertThat(msg1Doc.getFields().size()).isEqualTo(30); msg2Doc = convertAndDuplicateFieldDocBuilder.fromMessage(doc2); - assertThat(msg2Doc.getFields().size()).isEqualTo(30); + assertThat(msg2Doc.getFields().size()).isEqualTo(32); String additionalCreatedFieldName = makeNewFieldOfType(conflictingFieldName, FieldType.KEYWORD); // Value converted and new field is added. assertThat(getFieldCount(msg2Doc, Set.of(conflictingFieldName, additionalCreatedFieldName))) .isEqualTo(4); msg3Doc = convertAndDuplicateFieldDocBuilder.fromMessage(doc3); - assertThat(msg3Doc.getFields().size()).isEqualTo(30); + assertThat(msg3Doc.getFields().size()).isEqualTo(32); additionalCreatedFieldName = makeNewFieldOfType(conflictingFieldName, FieldType.INTEGER); // Value converted and new field is added. assertThat(getFieldCount(msg3Doc, Set.of(conflictingFieldName, additionalCreatedFieldName))) diff --git a/astra/src/test/java/com/slack/astra/logstore/search/AlreadyClosedLogIndexSearcherImpl.java b/astra/src/test/java/com/slack/astra/logstore/search/AlreadyClosedLogIndexSearcherImpl.java index aa8aafd013..4b06db4e9a 100644 --- a/astra/src/test/java/com/slack/astra/logstore/search/AlreadyClosedLogIndexSearcherImpl.java +++ b/astra/src/test/java/com/slack/astra/logstore/search/AlreadyClosedLogIndexSearcherImpl.java @@ -3,6 +3,7 @@ import com.slack.astra.logstore.LogMessage; import com.slack.astra.logstore.search.aggregations.AggBuilder; import org.apache.lucene.store.AlreadyClosedException; +import org.opensearch.index.query.QueryBuilder; public class AlreadyClosedLogIndexSearcherImpl implements LogIndexSearcher { @Override @@ -12,7 +13,8 @@ public SearchResult search( Long minTime, Long maxTime, int howMany, - AggBuilder aggBuilder) { + AggBuilder aggBuilder, + QueryBuilder queryBuilder) { throw new AlreadyClosedException("Failed to acquire an index searcher"); } diff --git a/astra/src/test/java/com/slack/astra/logstore/search/IllegalArgumentLogIndexSearcherImpl.java b/astra/src/test/java/com/slack/astra/logstore/search/IllegalArgumentLogIndexSearcherImpl.java index bd41cde1e6..168ccb4d8d 100644 --- a/astra/src/test/java/com/slack/astra/logstore/search/IllegalArgumentLogIndexSearcherImpl.java +++ b/astra/src/test/java/com/slack/astra/logstore/search/IllegalArgumentLogIndexSearcherImpl.java @@ -2,6 +2,7 @@ import com.slack.astra.logstore.LogMessage; import com.slack.astra.logstore.search.aggregations.AggBuilder; +import org.opensearch.index.query.QueryBuilder; public class IllegalArgumentLogIndexSearcherImpl implements LogIndexSearcher { @Override @@ -11,7 +12,8 @@ public SearchResult search( Long minTime, Long maxTime, int howMany, - AggBuilder aggBuilder) { + AggBuilder aggBuilder, + QueryBuilder queryBuilder) { throw new IllegalArgumentException("Failed to acquire an index searcher"); } diff --git a/astra/src/test/java/com/slack/astra/logstore/search/LogIndexSearcherImplTest.java b/astra/src/test/java/com/slack/astra/logstore/search/LogIndexSearcherImplTest.java index e577dfab6f..8e04f2c5c1 100644 --- a/astra/src/test/java/com/slack/astra/logstore/search/LogIndexSearcherImplTest.java +++ b/astra/src/test/java/com/slack/astra/logstore/search/LogIndexSearcherImplTest.java @@ -103,7 +103,8 @@ public void testTimeBoundSearch() { time.plusSeconds(10).toEpochMilli(), 1000, new DateHistogramAggBuilder( - "1", LogMessage.SystemField.TIME_SINCE_EPOCH.fieldName, "1s")) + "1", LogMessage.SystemField.TIME_SINCE_EPOCH.fieldName, "1s"), + null) .hits .size()) .isEqualTo(1); @@ -119,7 +120,8 @@ public void testTimeBoundSearch() { time.plusSeconds(90).toEpochMilli(), 1000, new DateHistogramAggBuilder( - "1", LogMessage.SystemField.TIME_SINCE_EPOCH.fieldName, "1s")) + "1", LogMessage.SystemField.TIME_SINCE_EPOCH.fieldName, "1s"), + null) .hits .size()) .isEqualTo(1); @@ -135,7 +137,8 @@ public void testTimeBoundSearch() { time.plusSeconds(100).toEpochMilli(), 1000, new DateHistogramAggBuilder( - "1", LogMessage.SystemField.TIME_SINCE_EPOCH.fieldName, "1s")) + "1", LogMessage.SystemField.TIME_SINCE_EPOCH.fieldName, "1s"), + null) .hits .size()) .isEqualTo(2); @@ -151,7 +154,8 @@ public void testTimeBoundSearch() { time.plusSeconds(1000).toEpochMilli(), 1000, new DateHistogramAggBuilder( - "1", LogMessage.SystemField.TIME_SINCE_EPOCH.fieldName, "1s")) + "1", LogMessage.SystemField.TIME_SINCE_EPOCH.fieldName, "1s"), + null) .hits .size()) .isEqualTo(2); @@ -180,7 +184,8 @@ public void testIndexBoundSearch() { time.plusSeconds(10).toEpochMilli(), 100, new DateHistogramAggBuilder( - "1", LogMessage.SystemField.TIME_SINCE_EPOCH.fieldName, "1s")) + "1", LogMessage.SystemField.TIME_SINCE_EPOCH.fieldName, "1s"), + null) .hits .size()) .isEqualTo(1); @@ -195,7 +200,8 @@ public void testIndexBoundSearch() { time.plusSeconds(10).toEpochMilli(), 100, new DateHistogramAggBuilder( - "1", LogMessage.SystemField.TIME_SINCE_EPOCH.fieldName, "1s")) + "1", LogMessage.SystemField.TIME_SINCE_EPOCH.fieldName, "1s"), + null) .hits .size()) .isEqualTo(1); @@ -210,7 +216,8 @@ public void testIndexBoundSearch() { time.plusSeconds(10).toEpochMilli(), 100, new DateHistogramAggBuilder( - "1", LogMessage.SystemField.TIME_SINCE_EPOCH.fieldName, "1s")) + "1", LogMessage.SystemField.TIME_SINCE_EPOCH.fieldName, "1s"), + null) .hits .size()) .isEqualTo(0); @@ -225,7 +232,8 @@ public void testIndexBoundSearch() { time.plusSeconds(10).toEpochMilli(), 100, new DateHistogramAggBuilder( - "1", LogMessage.SystemField.TIME_SINCE_EPOCH.fieldName, "1s")) + "1", LogMessage.SystemField.TIME_SINCE_EPOCH.fieldName, "1s"), + null) .hits .size()) .isEqualTo(0); @@ -243,7 +251,8 @@ public void testSearchMultipleItemsAndIndices() { time.plusSeconds(2).toEpochMilli(), 10, new DateHistogramAggBuilder( - "1", LogMessage.SystemField.TIME_SINCE_EPOCH.fieldName, "1s")); + "1", LogMessage.SystemField.TIME_SINCE_EPOCH.fieldName, "1s"), + null); assertThat(babies.hits.size()).isEqualTo(1); InternalDateHistogram histogram = @@ -275,7 +284,8 @@ public void testAllQueryWithFullTextSearchEnabled() { time.plusSeconds(2).toEpochMilli(), 10, new DateHistogramAggBuilder( - "1", LogMessage.SystemField.TIME_SINCE_EPOCH.fieldName, "1s")); + "1", LogMessage.SystemField.TIME_SINCE_EPOCH.fieldName, "1s"), + null); assertThat(termQuery.hits.size()).isEqualTo(1); SearchResult noTermStrQuery = @@ -286,7 +296,8 @@ public void testAllQueryWithFullTextSearchEnabled() { time.plusSeconds(2).toEpochMilli(), 10, new DateHistogramAggBuilder( - "1", LogMessage.SystemField.TIME_SINCE_EPOCH.fieldName, "1s")); + "1", LogMessage.SystemField.TIME_SINCE_EPOCH.fieldName, "1s"), + null); assertThat(noTermStrQuery.hits.size()).isEqualTo(1); SearchResult noTermNumericQuery = @@ -297,7 +308,8 @@ public void testAllQueryWithFullTextSearchEnabled() { time.plusSeconds(2).toEpochMilli(), 10, new DateHistogramAggBuilder( - "1", LogMessage.SystemField.TIME_SINCE_EPOCH.fieldName, "1s")); + "1", LogMessage.SystemField.TIME_SINCE_EPOCH.fieldName, "1s"), + null); assertThat(noTermNumericQuery.hits.size()).isEqualTo(1); } @@ -323,7 +335,8 @@ public void testAllQueryWithFullTextSearchDisabled() { time.plusSeconds(2).toEpochMilli(), 10, new DateHistogramAggBuilder( - "1", LogMessage.SystemField.TIME_SINCE_EPOCH.fieldName, "1s")); + "1", LogMessage.SystemField.TIME_SINCE_EPOCH.fieldName, "1s"), + null); assertThat(termQuery.hits.size()).isEqualTo(1); SearchResult noTermStrQuery = @@ -334,7 +347,8 @@ public void testAllQueryWithFullTextSearchDisabled() { time.plusSeconds(2).toEpochMilli(), 10, new DateHistogramAggBuilder( - "1", LogMessage.SystemField.TIME_SINCE_EPOCH.fieldName, "1s")); + "1", LogMessage.SystemField.TIME_SINCE_EPOCH.fieldName, "1s"), + null); assertThat(noTermStrQuery.hits.size()).isEqualTo(1); SearchResult noTermNumericQuery = @@ -345,7 +359,8 @@ public void testAllQueryWithFullTextSearchDisabled() { time.plusSeconds(2).toEpochMilli(), 10, new DateHistogramAggBuilder( - "1", LogMessage.SystemField.TIME_SINCE_EPOCH.fieldName, "1s")); + "1", LogMessage.SystemField.TIME_SINCE_EPOCH.fieldName, "1s"), + null); assertThat(noTermNumericQuery.hits.size()).isEqualTo(1); } @@ -377,7 +392,8 @@ public void testExistsQuery() { time.plusSeconds(2).toEpochMilli(), 10, new DateHistogramAggBuilder( - "1", LogMessage.SystemField.TIME_SINCE_EPOCH.fieldName, "1s")); + "1", LogMessage.SystemField.TIME_SINCE_EPOCH.fieldName, "1s"), + null); assertThat(exists.hits.size()).isEqualTo(1); SearchResult termQuery = @@ -388,7 +404,8 @@ public void testExistsQuery() { time.plusSeconds(2).toEpochMilli(), 10, new DateHistogramAggBuilder( - "1", LogMessage.SystemField.TIME_SINCE_EPOCH.fieldName, "1s")); + "1", LogMessage.SystemField.TIME_SINCE_EPOCH.fieldName, "1s"), + null); assertThat(termQuery.hits.size()).isEqualTo(1); SearchResult notExists = @@ -399,7 +416,8 @@ public void testExistsQuery() { time.plusSeconds(2).toEpochMilli(), 10, new DateHistogramAggBuilder( - "1", LogMessage.SystemField.TIME_SINCE_EPOCH.fieldName, "1s")); + "1", LogMessage.SystemField.TIME_SINCE_EPOCH.fieldName, "1s"), + null); assertThat(notExists.hits.size()).isEqualTo(0); } @@ -439,7 +457,8 @@ public void testRangeQuery() { time.plusSeconds(2).toEpochMilli(), 10, new DateHistogramAggBuilder( - "1", LogMessage.SystemField.TIME_SINCE_EPOCH.fieldName, "1s")); + "1", LogMessage.SystemField.TIME_SINCE_EPOCH.fieldName, "1s"), + null); assertThat(rangeBoundInclusive.hits.size()).isEqualTo(3); SearchResult rangeBoundExclusive = @@ -450,7 +469,8 @@ public void testRangeQuery() { time.plusSeconds(2).toEpochMilli(), 10, new DateHistogramAggBuilder( - "1", LogMessage.SystemField.TIME_SINCE_EPOCH.fieldName, "1s")); + "1", LogMessage.SystemField.TIME_SINCE_EPOCH.fieldName, "1s"), + null); assertThat(rangeBoundExclusive.hits.size()).isEqualTo(1); } @@ -507,7 +527,8 @@ public void testQueryParsingFieldTypes() { time.plusSeconds(2).toEpochMilli(), 10, new DateHistogramAggBuilder( - "1", LogMessage.SystemField.TIME_SINCE_EPOCH.fieldName, "1s")); + "1", LogMessage.SystemField.TIME_SINCE_EPOCH.fieldName, "1s"), + null); assertThat(boolquery.hits.size()).isEqualTo(1); SearchResult intquery = @@ -518,7 +539,8 @@ public void testQueryParsingFieldTypes() { time.plusSeconds(2).toEpochMilli(), 10, new DateHistogramAggBuilder( - "1", LogMessage.SystemField.TIME_SINCE_EPOCH.fieldName, "1s")); + "1", LogMessage.SystemField.TIME_SINCE_EPOCH.fieldName, "1s"), + null); assertThat(intquery.hits.size()).isEqualTo(1); SearchResult longquery = @@ -529,7 +551,8 @@ public void testQueryParsingFieldTypes() { time.plusSeconds(2).toEpochMilli(), 10, new DateHistogramAggBuilder( - "1", LogMessage.SystemField.TIME_SINCE_EPOCH.fieldName, "1s")); + "1", LogMessage.SystemField.TIME_SINCE_EPOCH.fieldName, "1s"), + null); assertThat(longquery.hits.size()).isEqualTo(1); SearchResult floatquery = @@ -540,7 +563,8 @@ public void testQueryParsingFieldTypes() { time.plusSeconds(2).toEpochMilli(), 10, new DateHistogramAggBuilder( - "1", LogMessage.SystemField.TIME_SINCE_EPOCH.fieldName, "1s")); + "1", LogMessage.SystemField.TIME_SINCE_EPOCH.fieldName, "1s"), + null); assertThat(floatquery.hits.size()).isEqualTo(1); SearchResult doublequery = @@ -551,7 +575,8 @@ public void testQueryParsingFieldTypes() { time.plusSeconds(2).toEpochMilli(), 10, new DateHistogramAggBuilder( - "1", LogMessage.SystemField.TIME_SINCE_EPOCH.fieldName, "1s")); + "1", LogMessage.SystemField.TIME_SINCE_EPOCH.fieldName, "1s"), + null); assertThat(doublequery.hits.size()).isEqualTo(1); } @@ -568,7 +593,8 @@ public void testTopKQuery() { time.plusSeconds(100).toEpochMilli(), 2, new DateHistogramAggBuilder( - "1", LogMessage.SystemField.TIME_SINCE_EPOCH.fieldName, "1s")); + "1", LogMessage.SystemField.TIME_SINCE_EPOCH.fieldName, "1s"), + null); assertThat(apples.hits.stream().map(m -> m.getId()).collect(Collectors.toList())) .isEqualTo(Arrays.asList("Message5", "Message3")); assertThat(apples.hits.size()).isEqualTo(2); @@ -599,7 +625,8 @@ public void testSearchMultipleCommits() { time.plusSeconds(10).toEpochMilli(), 2, new DateHistogramAggBuilder( - "1", LogMessage.SystemField.TIME_SINCE_EPOCH.fieldName, "1s")); + "1", LogMessage.SystemField.TIME_SINCE_EPOCH.fieldName, "1s"), + null); assertThat(baby.hits.size()).isEqualTo(1); assertThat(baby.hits.get(0).getId()).isEqualTo("Message2"); assertThat(getCount(MESSAGES_RECEIVED_COUNTER, strictLogStore.metricsRegistry)).isEqualTo(2); @@ -621,7 +648,8 @@ public void testSearchMultipleCommits() { time.toEpochMilli(), time.plusSeconds(10).toEpochMilli(), 2, - new DateHistogramAggBuilder("1", LogMessage.SystemField.TIME_SINCE_EPOCH.fieldName, "1s")); + new DateHistogramAggBuilder("1", LogMessage.SystemField.TIME_SINCE_EPOCH.fieldName, "1s"), + null); // Commit but no refresh. Item is still not available for search. strictLogStore.logStore.commit(); @@ -637,7 +665,8 @@ public void testSearchMultipleCommits() { time.toEpochMilli(), time.plusSeconds(10).toEpochMilli(), 2, - new DateHistogramAggBuilder("1", LogMessage.SystemField.TIME_SINCE_EPOCH.fieldName, "1s")); + new DateHistogramAggBuilder("1", LogMessage.SystemField.TIME_SINCE_EPOCH.fieldName, "1s"), + null); // Car can be searched after refresh. strictLogStore.logStore.refresh(); @@ -653,7 +682,8 @@ public void testSearchMultipleCommits() { time.toEpochMilli(), time.plusSeconds(10).toEpochMilli(), 2, - new DateHistogramAggBuilder("1", LogMessage.SystemField.TIME_SINCE_EPOCH.fieldName, "1s")); + new DateHistogramAggBuilder("1", LogMessage.SystemField.TIME_SINCE_EPOCH.fieldName, "1s"), + null); // Add another message to search, refresh but don't commit. strictLogStore.logStore.addMessage(SpanUtil.makeSpan(4, "apple baby car", time.plusSeconds(4))); @@ -673,7 +703,8 @@ public void testSearchMultipleCommits() { time.plusSeconds(10).toEpochMilli(), 2, new DateHistogramAggBuilder( - "1", LogMessage.SystemField.TIME_SINCE_EPOCH.fieldName, "1s")); + "1", LogMessage.SystemField.TIME_SINCE_EPOCH.fieldName, "1s"), + null); assertThat(babies.hits.size()).isEqualTo(2); assertThat(babies.hits.stream().map(m -> m.getId()).collect(Collectors.toList())) .isEqualTo(Arrays.asList("Message4", "Message2")); @@ -698,7 +729,8 @@ public void testFullIndexSearch() { MAX_TIME, 1000, new DateHistogramAggBuilder( - "1", LogMessage.SystemField.TIME_SINCE_EPOCH.fieldName, "1s")); + "1", LogMessage.SystemField.TIME_SINCE_EPOCH.fieldName, "1s"), + null); assertThat(allIndexItems.hits.size()).isEqualTo(4); @@ -725,7 +757,8 @@ public void testAggregationWithScripting() { 0L, MAX_TIME, 1000, - new AvgAggBuilder("1", TEST_SOURCE_LONG_PROPERTY, 0, null)); + new AvgAggBuilder("1", TEST_SOURCE_LONG_PROPERTY, 0, null), + null); assertThat(((InternalAvg) scriptNull.internalAggregation).value()).isEqualTo(3.25); SearchResult scriptEmpty = @@ -735,7 +768,8 @@ public void testAggregationWithScripting() { 0L, MAX_TIME, 1000, - new AvgAggBuilder("1", TEST_SOURCE_LONG_PROPERTY, 0, "")); + new AvgAggBuilder("1", TEST_SOURCE_LONG_PROPERTY, 0, ""), + null); assertThat(((InternalAvg) scriptEmpty.internalAggregation).value()).isEqualTo(3.25); SearchResult scripted = @@ -745,7 +779,8 @@ public void testAggregationWithScripting() { 0L, MAX_TIME, 1000, - new AvgAggBuilder("1", TEST_SOURCE_LONG_PROPERTY, 0, "return 9;")); + new AvgAggBuilder("1", TEST_SOURCE_LONG_PROPERTY, 0, "return 9;"), + null); assertThat(((InternalAvg) scripted.internalAggregation).value()).isEqualTo(9); } @@ -778,7 +813,8 @@ public void testFilterAggregations() { "%s:>%s", LogMessage.SystemField.TIME_SINCE_EPOCH.fieldName, time.plusSeconds(2).toEpochMilli()), - true)))); + true))), + null); assertThat(((InternalFilters) scriptNull.internalAggregation).getBuckets().size()).isEqualTo(2); assertThat(((InternalFilters) scriptNull.internalAggregation).getBuckets().get(0).getDocCount()) @@ -806,8 +842,8 @@ public void testFullIndexSearchForMinAgg() { 0L, MAX_TIME, 1000, - new MinAggBuilder( - "test", LogMessage.SystemField.TIME_SINCE_EPOCH.fieldName, "0", null)); + new MinAggBuilder("test", LogMessage.SystemField.TIME_SINCE_EPOCH.fieldName, "0", null), + null); assertThat(allIndexItems.hits.size()).isEqualTo(4); @@ -829,8 +865,8 @@ public void testFullIndexSearchForMaxAgg() { 0L, MAX_TIME, 1000, - new MaxAggBuilder( - "test", LogMessage.SystemField.TIME_SINCE_EPOCH.fieldName, "0", null)); + new MaxAggBuilder("test", LogMessage.SystemField.TIME_SINCE_EPOCH.fieldName, "0", null), + null); assertThat(allIndexItems.hits.size()).isEqualTo(4); @@ -854,7 +890,8 @@ public void testFullIndexSearchForSumAgg() { 0L, MAX_TIME, 1000, - new SumAggBuilder("test", TEST_SOURCE_LONG_PROPERTY, "0", null)); + new SumAggBuilder("test", TEST_SOURCE_LONG_PROPERTY, "0", null), + null); assertThat(allIndexItems.hits.size()).isEqualTo(4); @@ -877,7 +914,8 @@ public void testFullIndexSearchForExtendedStatsAgg() { 0L, MAX_TIME, 1000, - new ExtendedStatsAggBuilder("test", TEST_SOURCE_LONG_PROPERTY, "0", null, null)); + new ExtendedStatsAggBuilder("test", TEST_SOURCE_LONG_PROPERTY, "0", null, null), + null); assertThat(allIndexItems.hits.size()).isEqualTo(4); @@ -908,13 +946,8 @@ public void testTermsAggregation() { MAX_TIME, 1000, new TermsAggBuilder( - "1", - List.of(), - TEST_SOURCE_STRING_PROPERTY, - "foo", - 10, - 0, - Map.of("_count", "asc"))); + "1", List.of(), TEST_SOURCE_STRING_PROPERTY, "foo", 10, 0, Map.of("_count", "asc")), + null); assertThat(allIndexItems.hits.size()).isEqualTo(4); @@ -959,7 +992,8 @@ public void testPipelineAggregation() { null, null), new MovingAvgAggBuilder("movAvgCount", "_count", "simple", 2, 1))), - List.of()); + List.of(), + null); SearchResult allIndexItems = strictLogStore.logSearcher.search( @@ -968,7 +1002,8 @@ public void testPipelineAggregation() { query.startTimeEpochMs, query.endTimeEpochMs, query.howMany, - query.aggBuilder); + query.aggBuilder, + query.queryBuilder); SearchResultAggregatorImpl aggregator = new SearchResultAggregatorImpl<>(query); SearchResult allIndexItemsFinal = @@ -1016,7 +1051,8 @@ public void testTermsAggregationMissingValues() { MAX_TIME, 1000, new TermsAggBuilder( - "1", List.of(), "thisFieldDoesNotExist", "foo", 10, 0, Map.of("_count", "asc"))); + "1", List.of(), "thisFieldDoesNotExist", "foo", 10, 0, Map.of("_count", "asc")), + null); assertThat(allIndexItems.hits.size()).isEqualTo(4); @@ -1050,7 +1086,8 @@ public void testFullTextSearch() { MAX_TIME, 1000, new DateHistogramAggBuilder( - "1", LogMessage.SystemField.TIME_SINCE_EPOCH.fieldName, "1s")) + "1", LogMessage.SystemField.TIME_SINCE_EPOCH.fieldName, "1s"), + null) .hits .size()) .isEqualTo(1); @@ -1065,7 +1102,8 @@ public void testFullTextSearch() { MAX_TIME, 1000, new DateHistogramAggBuilder( - "1", LogMessage.SystemField.TIME_SINCE_EPOCH.fieldName, "1s")) + "1", LogMessage.SystemField.TIME_SINCE_EPOCH.fieldName, "1s"), + null) .hits .size()) .isEqualTo(1); @@ -1085,7 +1123,8 @@ public void testFullTextSearch() { MAX_TIME, 1000, new DateHistogramAggBuilder( - "1", LogMessage.SystemField.TIME_SINCE_EPOCH.fieldName, "1s")) + "1", LogMessage.SystemField.TIME_SINCE_EPOCH.fieldName, "1s"), + null) .hits .size()) .isEqualTo(1); @@ -1099,7 +1138,8 @@ public void testFullTextSearch() { MAX_TIME, 1000, new DateHistogramAggBuilder( - "1", LogMessage.SystemField.TIME_SINCE_EPOCH.fieldName, "1s")) + "1", LogMessage.SystemField.TIME_SINCE_EPOCH.fieldName, "1s"), + null) .hits .size()) .isEqualTo(2); @@ -1114,7 +1154,8 @@ public void testFullTextSearch() { MAX_TIME, 1000, new DateHistogramAggBuilder( - "1", LogMessage.SystemField.TIME_SINCE_EPOCH.fieldName, "1s")) + "1", LogMessage.SystemField.TIME_SINCE_EPOCH.fieldName, "1s"), + null) .hits .size()) .isEqualTo(1); @@ -1128,7 +1169,8 @@ public void testFullTextSearch() { MAX_TIME, 1000, new DateHistogramAggBuilder( - "1", LogMessage.SystemField.TIME_SINCE_EPOCH.fieldName, "1s")) + "1", LogMessage.SystemField.TIME_SINCE_EPOCH.fieldName, "1s"), + null) .hits .size()) .isEqualTo(2); @@ -1147,7 +1189,8 @@ public void testFullTextSearch() { MAX_TIME, 1000, new DateHistogramAggBuilder( - "1", LogMessage.SystemField.TIME_SINCE_EPOCH.fieldName, "1s")) + "1", LogMessage.SystemField.TIME_SINCE_EPOCH.fieldName, "1s"), + null) .hits .size()) .isEqualTo(2); @@ -1161,7 +1204,8 @@ public void testFullTextSearch() { MAX_TIME, 1000, new DateHistogramAggBuilder( - "1", LogMessage.SystemField.TIME_SINCE_EPOCH.fieldName, "1s")) + "1", LogMessage.SystemField.TIME_SINCE_EPOCH.fieldName, "1s"), + null) .hits .size()) .isEqualTo(3); @@ -1176,7 +1220,8 @@ public void testFullTextSearch() { MAX_TIME, 1000, new DateHistogramAggBuilder( - "1", LogMessage.SystemField.TIME_SINCE_EPOCH.fieldName, "1s")) + "1", LogMessage.SystemField.TIME_SINCE_EPOCH.fieldName, "1s"), + null) .hits .size()) .isEqualTo(2); @@ -1190,7 +1235,8 @@ public void testFullTextSearch() { MAX_TIME, 1000, new DateHistogramAggBuilder( - "1", LogMessage.SystemField.TIME_SINCE_EPOCH.fieldName, "1s")) + "1", LogMessage.SystemField.TIME_SINCE_EPOCH.fieldName, "1s"), + null) .hits .size()) .isEqualTo(3); @@ -1206,7 +1252,8 @@ public void testFullTextSearch() { MAX_TIME, 1000, new DateHistogramAggBuilder( - "1", LogMessage.SystemField.TIME_SINCE_EPOCH.fieldName, "1s")) + "1", LogMessage.SystemField.TIME_SINCE_EPOCH.fieldName, "1s"), + null) .hits .size()) .isEqualTo(3); @@ -1221,7 +1268,8 @@ public void testFullTextSearch() { MAX_TIME, 1000, new DateHistogramAggBuilder( - "1", LogMessage.SystemField.TIME_SINCE_EPOCH.fieldName, "1s")) + "1", LogMessage.SystemField.TIME_SINCE_EPOCH.fieldName, "1s"), + null) .hits .size()) .isEqualTo(2); @@ -1237,7 +1285,8 @@ public void testFullTextSearch() { MAX_TIME, 1000, new DateHistogramAggBuilder( - "1", LogMessage.SystemField.TIME_SINCE_EPOCH.fieldName, "1s")) + "1", LogMessage.SystemField.TIME_SINCE_EPOCH.fieldName, "1s"), + null) .hits .size()) .isEqualTo(2); @@ -1253,7 +1302,8 @@ public void testFullTextSearch() { MAX_TIME, 1000, new DateHistogramAggBuilder( - "1", LogMessage.SystemField.TIME_SINCE_EPOCH.fieldName, "1s")) + "1", LogMessage.SystemField.TIME_SINCE_EPOCH.fieldName, "1s"), + null) .hits .size()) .isEqualTo(3); @@ -1268,7 +1318,8 @@ public void testFullTextSearch() { MAX_TIME, 1000, new DateHistogramAggBuilder( - "1", LogMessage.SystemField.TIME_SINCE_EPOCH.fieldName, "1s")) + "1", LogMessage.SystemField.TIME_SINCE_EPOCH.fieldName, "1s"), + null) .hits .size()) .isEqualTo(0); @@ -1305,7 +1356,8 @@ public void testDisabledFullTextSearch() { MAX_TIME, 1000, new DateHistogramAggBuilder( - "1", LogMessage.SystemField.TIME_SINCE_EPOCH.fieldName, "1s")) + "1", LogMessage.SystemField.TIME_SINCE_EPOCH.fieldName, "1s"), + null) .hits .size()) .isZero(); @@ -1320,7 +1372,8 @@ public void testDisabledFullTextSearch() { MAX_TIME, 1000, new DateHistogramAggBuilder( - "1", LogMessage.SystemField.TIME_SINCE_EPOCH.fieldName, "1s")) + "1", LogMessage.SystemField.TIME_SINCE_EPOCH.fieldName, "1s"), + null) .hits .size()) .isZero(); @@ -1337,7 +1390,8 @@ public void testDisabledFullTextSearch() { MAX_TIME, 1000, new DateHistogramAggBuilder( - "1", LogMessage.SystemField.TIME_SINCE_EPOCH.fieldName, "1s")) + "1", LogMessage.SystemField.TIME_SINCE_EPOCH.fieldName, "1s"), + null) .hits .size()) .isEqualTo(1); @@ -1352,7 +1406,8 @@ public void testDisabledFullTextSearch() { MAX_TIME, 1000, new DateHistogramAggBuilder( - "1", LogMessage.SystemField.TIME_SINCE_EPOCH.fieldName, "1s")) + "1", LogMessage.SystemField.TIME_SINCE_EPOCH.fieldName, "1s"), + null) .hits .size()) .isEqualTo(0); @@ -1367,7 +1422,8 @@ public void testDisabledFullTextSearch() { MAX_TIME, 1000, new DateHistogramAggBuilder( - "1", LogMessage.SystemField.TIME_SINCE_EPOCH.fieldName, "1s")) + "1", LogMessage.SystemField.TIME_SINCE_EPOCH.fieldName, "1s"), + null) .hits .size()) .isEqualTo(2); @@ -1383,7 +1439,8 @@ public void testDisabledFullTextSearch() { MAX_TIME, 1000, new DateHistogramAggBuilder( - "1", LogMessage.SystemField.TIME_SINCE_EPOCH.fieldName, "1s")) + "1", LogMessage.SystemField.TIME_SINCE_EPOCH.fieldName, "1s"), + null) .hits .size()) .isZero(); @@ -1398,7 +1455,8 @@ public void testDisabledFullTextSearch() { MAX_TIME, 1000, new DateHistogramAggBuilder( - "1", LogMessage.SystemField.TIME_SINCE_EPOCH.fieldName, "1s")) + "1", LogMessage.SystemField.TIME_SINCE_EPOCH.fieldName, "1s"), + null) .hits .size()) .isZero(); @@ -1414,7 +1472,8 @@ public void testDisabledFullTextSearch() { MAX_TIME, 1000, new DateHistogramAggBuilder( - "1", LogMessage.SystemField.TIME_SINCE_EPOCH.fieldName, "1s")) + "1", LogMessage.SystemField.TIME_SINCE_EPOCH.fieldName, "1s"), + null) .hits .size()) .isZero(); @@ -1430,7 +1489,8 @@ public void testDisabledFullTextSearch() { MAX_TIME, 1000, new DateHistogramAggBuilder( - "1", LogMessage.SystemField.TIME_SINCE_EPOCH.fieldName, "1s")) + "1", LogMessage.SystemField.TIME_SINCE_EPOCH.fieldName, "1s"), + null) .hits .size()) .isZero(); @@ -1445,7 +1505,8 @@ public void testDisabledFullTextSearch() { MAX_TIME, 1000, new DateHistogramAggBuilder( - "1", LogMessage.SystemField.TIME_SINCE_EPOCH.fieldName, "1s")) + "1", LogMessage.SystemField.TIME_SINCE_EPOCH.fieldName, "1s"), + null) .hits .size()) .isZero(); @@ -1466,7 +1527,8 @@ public void testNullSearchString() { MAX_TIME, 1000, new DateHistogramAggBuilder( - "1", LogMessage.SystemField.TIME_SINCE_EPOCH.fieldName, "1s"))); + "1", LogMessage.SystemField.TIME_SINCE_EPOCH.fieldName, "1s"), + null)); } @Test @@ -1483,7 +1545,8 @@ public void testMissingIndexSearch() { MAX_TIME, 1000, new DateHistogramAggBuilder( - "1", LogMessage.SystemField.TIME_SINCE_EPOCH.fieldName, "1s")); + "1", LogMessage.SystemField.TIME_SINCE_EPOCH.fieldName, "1s"), + null); assertThat(allIndexItems.hits.size()).isEqualTo(0); @@ -1511,7 +1574,8 @@ public void testNoResultQuery() { MAX_TIME, 1000, new DateHistogramAggBuilder( - "1", LogMessage.SystemField.TIME_SINCE_EPOCH.fieldName, "1s")); + "1", LogMessage.SystemField.TIME_SINCE_EPOCH.fieldName, "1s"), + null); assertThat(elephants.hits.size()).isEqualTo(0); InternalDateHistogram histogram = @@ -1530,6 +1594,7 @@ public void testSearchAndNoStats() { time.toEpochMilli(), time.plusSeconds(10).toEpochMilli(), 100, + null, null); assertThat(results.hits.size()).isEqualTo(2); assertThat(results.internalAggregation).isNull(); @@ -1547,7 +1612,8 @@ public void testSearchOnlyHistogram() { time.plusSeconds(10).toEpochMilli(), 0, new DateHistogramAggBuilder( - "1", LogMessage.SystemField.TIME_SINCE_EPOCH.fieldName, "1s")); + "1", LogMessage.SystemField.TIME_SINCE_EPOCH.fieldName, "1s"), + null); assertThat(babies.hits.size()).isEqualTo(0); InternalDateHistogram histogram = @@ -1580,7 +1646,8 @@ public void testEmptyIndexName() { MAX_TIME, 1000, new DateHistogramAggBuilder( - "1", LogMessage.SystemField.TIME_SINCE_EPOCH.fieldName, "1s"))); + "1", LogMessage.SystemField.TIME_SINCE_EPOCH.fieldName, "1s"), + null)); } @Test @@ -1597,7 +1664,8 @@ public void testNullIndexName() { MAX_TIME, 1000, new DateHistogramAggBuilder( - "1", LogMessage.SystemField.TIME_SINCE_EPOCH.fieldName, "1s"))); + "1", LogMessage.SystemField.TIME_SINCE_EPOCH.fieldName, "1s"), + null)); } @Test @@ -1614,7 +1682,8 @@ public void testInvalidStartTime() { MAX_TIME, 1000, new DateHistogramAggBuilder( - "1", LogMessage.SystemField.TIME_SINCE_EPOCH.fieldName, "1s"))); + "1", LogMessage.SystemField.TIME_SINCE_EPOCH.fieldName, "1s"), + null)); } @Test @@ -1631,7 +1700,8 @@ public void testInvalidEndTime() { -1L, 1000, new DateHistogramAggBuilder( - "1", LogMessage.SystemField.TIME_SINCE_EPOCH.fieldName, "1s"))); + "1", LogMessage.SystemField.TIME_SINCE_EPOCH.fieldName, "1s"), + null)); } @Test @@ -1648,7 +1718,8 @@ public void testInvalidTimeRange() { time.minusSeconds(1).toEpochMilli(), 1000, new DateHistogramAggBuilder( - "1", LogMessage.SystemField.TIME_SINCE_EPOCH.fieldName, "1s"))); + "1", LogMessage.SystemField.TIME_SINCE_EPOCH.fieldName, "1s"), + null)); } @Test @@ -1664,6 +1735,7 @@ public void testSearchOrHistogramQuery() { time.toEpochMilli(), time.plusSeconds(1).toEpochMilli(), 0, + null, null)); } @@ -1681,7 +1753,8 @@ public void testNegativeHitCount() { time.plusSeconds(1).toEpochMilli(), -1, new DateHistogramAggBuilder( - "1", LogMessage.SystemField.TIME_SINCE_EPOCH.fieldName, "1s"))); + "1", LogMessage.SystemField.TIME_SINCE_EPOCH.fieldName, "1s"), + null)); } @Test @@ -1698,7 +1771,8 @@ public void testNegativeHistogramInterval() { time.plusSeconds(1).toEpochMilli(), 1, new DateHistogramAggBuilder( - "1", LogMessage.SystemField.TIME_SINCE_EPOCH.fieldName, "-1s"))); + "1", LogMessage.SystemField.TIME_SINCE_EPOCH.fieldName, "-1s"), + null)); } @Test @@ -1715,7 +1789,8 @@ public void testQueryParseError() { time.plusSeconds(1).toEpochMilli(), 1, new DateHistogramAggBuilder( - "1", LogMessage.SystemField.TIME_SINCE_EPOCH.fieldName, "1s"))); + "1", LogMessage.SystemField.TIME_SINCE_EPOCH.fieldName, "1s"), + null)); } @Test @@ -1740,7 +1815,8 @@ public void testConcurrentSearches() throws InterruptedException { MAX_TIME, 100, new DateHistogramAggBuilder( - "1", LogMessage.SystemField.TIME_SINCE_EPOCH.fieldName, "1s")); + "1", LogMessage.SystemField.TIME_SINCE_EPOCH.fieldName, "1s"), + null); if (babies.hits.size() != 2) { searchFailures.addAndGet(1); } else { @@ -1776,7 +1852,8 @@ public void testSearchById() { time.plusSeconds(2).toEpochMilli(), 10, new DateHistogramAggBuilder( - "1", LogMessage.SystemField.TIME_SINCE_EPOCH.fieldName, "1s")); + "1", LogMessage.SystemField.TIME_SINCE_EPOCH.fieldName, "1s"), + null); assertThat(index.hits.size()).isEqualTo(1); } } diff --git a/astra/src/test/java/com/slack/astra/logstore/search/SearchResultAggregatorImplTest.java b/astra/src/test/java/com/slack/astra/logstore/search/SearchResultAggregatorImplTest.java index 96f693b4c7..c5dcb1d9eb 100644 --- a/astra/src/test/java/com/slack/astra/logstore/search/SearchResultAggregatorImplTest.java +++ b/astra/src/test/java/com/slack/astra/logstore/search/SearchResultAggregatorImplTest.java @@ -81,7 +81,8 @@ public void testSimpleSearchResultsAggWithOneResult() throws IOException { howMany, new DateHistogramAggBuilder( "1", LogMessage.SystemField.TIME_SINCE_EPOCH.fieldName, "6m"), - Collections.emptyList()); + Collections.emptyList(), + null); List> searchResults = new ArrayList<>(2); searchResults.add(searchResult1); searchResults.add(searchResult2); @@ -151,7 +152,8 @@ public void testSimpleSearchResultsAggWithMultipleResults() throws IOException { howMany, new DateHistogramAggBuilder( "1", LogMessage.SystemField.TIME_SINCE_EPOCH.fieldName, "10m"), - Collections.emptyList()); + Collections.emptyList(), + null); List> searchResults = new ArrayList<>(2); searchResults.add(searchResult1); searchResults.add(searchResult2); @@ -243,7 +245,8 @@ public void testSearchResultAggregatorOn4Results() throws IOException { howMany, new DateHistogramAggBuilder( "1", LogMessage.SystemField.TIME_SINCE_EPOCH.fieldName, "6m"), - Collections.emptyList()); + Collections.emptyList(), + null); List> searchResults = List.of(searchResult1, searchResult4, searchResult3, searchResult2); SearchResult aggSearchResult = @@ -297,7 +300,8 @@ public void testSimpleSearchResultsAggWithNoHistograms() throws IOException { howMany, new DateHistogramAggBuilder( "1", LogMessage.SystemField.TIME_SINCE_EPOCH.fieldName, "6m"), - Collections.emptyList()); + Collections.emptyList(), + null); List> searchResults = new ArrayList<>(2); searchResults.add(searchResult1); searchResults.add(searchResult2); @@ -360,7 +364,8 @@ public void testSimpleSearchResultsAggNoHits() throws IOException { howMany, new DateHistogramAggBuilder( "1", LogMessage.SystemField.TIME_SINCE_EPOCH.fieldName, "6m"), - Collections.emptyList()); + Collections.emptyList(), + null); List> searchResults = new ArrayList<>(2); searchResults.add(searchResult1); searchResults.add(searchResult2); @@ -419,7 +424,8 @@ public void testSearchResultsAggIgnoresBucketsInSearchResultsSafely() throws IOE howMany, new DateHistogramAggBuilder( "1", LogMessage.SystemField.TIME_SINCE_EPOCH.fieldName, "6m"), - Collections.emptyList()); + Collections.emptyList(), + null); List> searchResults = new ArrayList<>(2); searchResults.add(searchResult1); searchResults.add(searchResult2); @@ -484,7 +490,8 @@ public void testSimpleSearchResultsAggIgnoreHitsSafely() throws IOException { howMany, new DateHistogramAggBuilder( "1", LogMessage.SystemField.TIME_SINCE_EPOCH.fieldName, "10m"), - Collections.emptyList()); + Collections.emptyList(), + null); List> searchResults = new ArrayList<>(2); searchResults.add(searchResult1); searchResults.add(searchResult2); @@ -556,7 +563,8 @@ private InternalAggregation makeHistogram( 0, "", Map.of("min", histogramStartMs, "max", histogramEndMs), - List.of())); + List.of()), + null); try { return messageSearchResult.internalAggregation; diff --git a/astra/src/test/java/com/slack/astra/logstore/search/SearchResultUtilsTest.java b/astra/src/test/java/com/slack/astra/logstore/search/SearchResultUtilsTest.java index 3231ccb8f0..fd00732163 100644 --- a/astra/src/test/java/com/slack/astra/logstore/search/SearchResultUtilsTest.java +++ b/astra/src/test/java/com/slack/astra/logstore/search/SearchResultUtilsTest.java @@ -29,9 +29,311 @@ import java.util.List; import java.util.Map; import org.junit.jupiter.api.Test; +import org.opensearch.index.query.BoolQueryBuilder; +import org.opensearch.index.query.IntervalQueryBuilder; +import org.opensearch.index.query.QueryStringQueryBuilder; +import org.opensearch.index.query.TermsQueryBuilder; public class SearchResultUtilsTest { + @Test + public void shouldParseBasicMustNotQueryIntoQueryBuilder() { + AstraSearch.SearchRequest searchRequest = + AstraSearch.SearchRequest.newBuilder() + .setQueryString("") + .setQuery( + """ + { + "bool": { + "must_not": { + "query_string": { + "query": "this is a test" + } + } + } + }""") + .build(); + SearchQuery output = SearchResultUtils.fromSearchRequest(searchRequest); + assertThat(output.queryStr).isEmpty(); + assertThat(output.queryBuilder).isInstanceOf(BoolQueryBuilder.class); + + BoolQueryBuilder boolQueryBuilder = (BoolQueryBuilder) output.queryBuilder; + assertThat(boolQueryBuilder.mustNot().size()).isEqualTo(1); + assertThat(boolQueryBuilder.mustNot().getFirst()).isInstanceOf(QueryStringQueryBuilder.class); + + QueryStringQueryBuilder queryStringQueryBuilder = + (QueryStringQueryBuilder) boolQueryBuilder.mustNot().getFirst(); + assertThat(queryStringQueryBuilder.queryString()).isEqualTo("this is a test"); + } + + @Test + public void shouldParseMustNotTermsQueryIntoQueryBuilder() { + AstraSearch.SearchRequest searchRequest = + AstraSearch.SearchRequest.newBuilder() + .setQueryString("") + .setQuery( + """ + { + "bool": { + "must_not": { + "terms": { + "test_field": [ + "this", + "is", + "a", + "test" + ] + } + } + } + }""") + .build(); + SearchQuery output = SearchResultUtils.fromSearchRequest(searchRequest); + assertThat(output.queryStr).isEmpty(); + assertThat(output.queryBuilder).isInstanceOf(BoolQueryBuilder.class); + + BoolQueryBuilder boolQueryBuilder = (BoolQueryBuilder) output.queryBuilder; + assertThat(boolQueryBuilder.mustNot().size()).isEqualTo(1); + assertThat(boolQueryBuilder.mustNot().getFirst()).isInstanceOf(TermsQueryBuilder.class); + + TermsQueryBuilder termsQueryBuilder = (TermsQueryBuilder) boolQueryBuilder.mustNot().getFirst(); + assertThat(termsQueryBuilder.fieldName()).isEqualTo("test_field"); + } + + @Test + public void shouldParseBasicFilterQueryIntoQueryBuilder() { + AstraSearch.SearchRequest searchRequest = + AstraSearch.SearchRequest.newBuilder() + .setQueryString("") + .setQuery( + """ + { + "bool": { + "filter": [{ + "query_string": { + "query": "this is a test" + } + }] + } + }""") + .build(); + SearchQuery output = SearchResultUtils.fromSearchRequest(searchRequest); + assertThat(output.queryStr).isEmpty(); + assertThat(output.queryBuilder).isInstanceOf(BoolQueryBuilder.class); + + BoolQueryBuilder boolQueryBuilder = (BoolQueryBuilder) output.queryBuilder; + assertThat(boolQueryBuilder.filter().size()).isEqualTo(1); + assertThat(boolQueryBuilder.filter().getFirst()).isInstanceOf(QueryStringQueryBuilder.class); + + QueryStringQueryBuilder queryStringQueryBuilder = + (QueryStringQueryBuilder) boolQueryBuilder.filter().getFirst(); + assertThat(queryStringQueryBuilder.queryString()).isEqualTo("this is a test"); + } + + @Test + public void shouldParseFilterTermsQueryIntoQueryBuilder() { + AstraSearch.SearchRequest searchRequest = + AstraSearch.SearchRequest.newBuilder() + .setQueryString("") + .setQuery( + """ + { + "bool": { + "filter": { + "terms": { + "test_field": [ + "this", + "is", + "a", + "test" + ] + } + } + } + }""") + .build(); + SearchQuery output = SearchResultUtils.fromSearchRequest(searchRequest); + assertThat(output.queryStr).isEmpty(); + assertThat(output.queryBuilder).isInstanceOf(BoolQueryBuilder.class); + + BoolQueryBuilder boolQueryBuilder = (BoolQueryBuilder) output.queryBuilder; + assertThat(boolQueryBuilder.filter().size()).isEqualTo(1); + assertThat(boolQueryBuilder.filter().getFirst()).isInstanceOf(TermsQueryBuilder.class); + + TermsQueryBuilder termsQueryBuilder = (TermsQueryBuilder) boolQueryBuilder.filter().getFirst(); + assertThat(termsQueryBuilder.fieldName()).isEqualTo("test_field"); + } + + @Test + public void shouldParseBasicMustQueryIntoQueryBuilder() { + AstraSearch.SearchRequest searchRequest = + AstraSearch.SearchRequest.newBuilder() + .setQueryString("") + .setQuery( + """ + { + "bool": { + "must": { + "query_string": { + "query": "this is a test" + } + } + } + }""") + .build(); + SearchQuery output = SearchResultUtils.fromSearchRequest(searchRequest); + assertThat(output.queryStr).isEmpty(); + assertThat(output.queryBuilder).isInstanceOf(BoolQueryBuilder.class); + + BoolQueryBuilder boolQueryBuilder = (BoolQueryBuilder) output.queryBuilder; + assertThat(boolQueryBuilder.must().size()).isEqualTo(1); + assertThat(boolQueryBuilder.must().getFirst()).isInstanceOf(QueryStringQueryBuilder.class); + + QueryStringQueryBuilder queryStringQueryBuilder = + (QueryStringQueryBuilder) boolQueryBuilder.must().getFirst(); + assertThat(queryStringQueryBuilder.queryString()).isEqualTo("this is a test"); + } + + @Test + public void shouldParseMustTermsQueryIntoQueryBuilder() { + AstraSearch.SearchRequest searchRequest = + AstraSearch.SearchRequest.newBuilder() + .setQueryString("") + .setQuery( + """ + { + "bool": { + "must": { + "terms": { + "test_field": [ + "this", + "is", + "a", + "test" + ] + } + } + } + }""") + .build(); + SearchQuery output = SearchResultUtils.fromSearchRequest(searchRequest); + assertThat(output.queryStr).isEmpty(); + assertThat(output.queryBuilder).isInstanceOf(BoolQueryBuilder.class); + + BoolQueryBuilder boolQueryBuilder = (BoolQueryBuilder) output.queryBuilder; + assertThat(boolQueryBuilder.must().size()).isEqualTo(1); + assertThat(boolQueryBuilder.must().getFirst()).isInstanceOf(TermsQueryBuilder.class); + + TermsQueryBuilder termsQueryBuilder = (TermsQueryBuilder) boolQueryBuilder.must().getFirst(); + assertThat(termsQueryBuilder.fieldName()).isEqualTo("test_field"); + } + + @Test + public void shouldParseBasicShouldQueryIntoQueryBuilder() { + AstraSearch.SearchRequest searchRequest = + AstraSearch.SearchRequest.newBuilder() + .setQueryString("") + .setQuery( + """ + { + "bool": { + "should": { + "query_string": { + "query": "this is a test" + } + } + } + }""") + .build(); + SearchQuery output = SearchResultUtils.fromSearchRequest(searchRequest); + assertThat(output.queryStr).isEmpty(); + assertThat(output.queryBuilder).isInstanceOf(BoolQueryBuilder.class); + + BoolQueryBuilder boolQueryBuilder = (BoolQueryBuilder) output.queryBuilder; + assertThat(boolQueryBuilder.should().size()).isEqualTo(1); + assertThat(boolQueryBuilder.should().getFirst()).isInstanceOf(QueryStringQueryBuilder.class); + + QueryStringQueryBuilder queryStringQueryBuilder = + (QueryStringQueryBuilder) boolQueryBuilder.should().getFirst(); + assertThat(queryStringQueryBuilder.queryString()).isEqualTo("this is a test"); + } + + @Test + public void shouldParseShouldTermsQueryIntoQueryBuilder() { + AstraSearch.SearchRequest searchRequest = + AstraSearch.SearchRequest.newBuilder() + .setQueryString("") + .setQuery( + """ + { + "bool": { + "should": { + "terms": { + "test_field": [ + "this", + "is", + "a", + "test" + ] + } + } + } + }""") + .build(); + SearchQuery output = SearchResultUtils.fromSearchRequest(searchRequest); + assertThat(output.queryStr).isEmpty(); + assertThat(output.queryBuilder).isInstanceOf(BoolQueryBuilder.class); + + BoolQueryBuilder boolQueryBuilder = (BoolQueryBuilder) output.queryBuilder; + assertThat(boolQueryBuilder.should().size()).isEqualTo(1); + assertThat(boolQueryBuilder.should().getFirst()).isInstanceOf(TermsQueryBuilder.class); + + TermsQueryBuilder termsQueryBuilder = (TermsQueryBuilder) boolQueryBuilder.should().getFirst(); + assertThat(termsQueryBuilder.fieldName()).isEqualTo("test_field"); + } + + @Test + public void shouldParseNonBoolQueryIntoQueryBuilder() { + AstraSearch.SearchRequest searchRequest = + AstraSearch.SearchRequest.newBuilder() + .setQueryString("") + .setQuery( + """ + { + "intervals" : { + "my_text" : { + "all_of" : { + "ordered" : true, + "intervals" : [ + { + "match" : { + "query" : "my favorite food", + "max_gaps" : 0, + "ordered" : true + } + }, + { + "any_of" : { + "intervals" : [ + { "match" : { "query" : "hot water" } }, + { "match" : { "query" : "cold porridge" } } + ] + } + } + ] + } + } + } + }""") + .build(); + SearchQuery output = SearchResultUtils.fromSearchRequest(searchRequest); + assertThat(output.queryStr).isEmpty(); + assertThat(output.queryBuilder).isInstanceOf(IntervalQueryBuilder.class); + + IntervalQueryBuilder intervalQueryBuilder = (IntervalQueryBuilder) output.queryBuilder; + assertThat(intervalQueryBuilder.getField()).isEqualTo("my_text"); + } + @Test public void shouldConvertMinAggToFromProto() { MinAggBuilder minAggBuilder = diff --git a/astra/src/test/java/com/slack/astra/logstore/search/StatsCollectorTest.java b/astra/src/test/java/com/slack/astra/logstore/search/StatsCollectorTest.java index 0a3d9189e0..c96e56547c 100644 --- a/astra/src/test/java/com/slack/astra/logstore/search/StatsCollectorTest.java +++ b/astra/src/test/java/com/slack/astra/logstore/search/StatsCollectorTest.java @@ -53,7 +53,8 @@ public void testStatsCollectorWithPerMinuteMessages() { time.plusSeconds(4 * 60).toEpochMilli(), 0, new DateHistogramAggBuilder( - "1", LogMessage.SystemField.TIME_SINCE_EPOCH.fieldName, "1s")); + "1", LogMessage.SystemField.TIME_SINCE_EPOCH.fieldName, "1s"), + null); assertThat(allIndexItems.hits.size()).isEqualTo(0); diff --git a/astra/src/test/java/com/slack/astra/server/AstraIndexerTest.java b/astra/src/test/java/com/slack/astra/server/AstraIndexerTest.java index bdeed5b79b..978d57378e 100644 --- a/astra/src/test/java/com/slack/astra/server/AstraIndexerTest.java +++ b/astra/src/test/java/com/slack/astra/server/AstraIndexerTest.java @@ -736,7 +736,8 @@ private void consumeMessagesAndSearchMessagesTest( 10, new DateHistogramAggBuilder( "1", LogMessage.SystemField.TIME_SINCE_EPOCH.fieldName, "1s"), - Collections.emptyList()), + Collections.emptyList(), + null), Duration.ofMillis(3000)); // Validate search response diff --git a/astra/src/test/java/com/slack/astra/testlib/SpanUtil.java b/astra/src/test/java/com/slack/astra/testlib/SpanUtil.java index fe2ef73564..8157b421f7 100644 --- a/astra/src/test/java/com/slack/astra/testlib/SpanUtil.java +++ b/astra/src/test/java/com/slack/astra/testlib/SpanUtil.java @@ -9,6 +9,7 @@ import static com.slack.astra.testlib.MessageUtil.TEST_SOURCE_STRING_PROPERTY; import com.google.protobuf.ByteString; +import com.google.protobuf.Timestamp; import com.slack.astra.logstore.LogMessage; import com.slack.astra.proto.schema.Schema; import com.slack.service.murron.trace.Trace; @@ -147,6 +148,15 @@ public static Trace.Span makeSpan( .setTimestamp( TimeUnit.MICROSECONDS.convert(timestamp.toEpochMilli(), TimeUnit.MILLISECONDS)) .setId(ByteString.copyFromUtf8(id)) + .addTags( + Trace.KeyValue.newBuilder() + .setVDate( + Timestamp.newBuilder() + .setSeconds(timestamp.getEpochSecond()) + .setNanos(timestamp.getNano())) + .setKey("@timestamp") + .setFieldType(Schema.SchemaFieldType.DATE) + .build()) .addTags( Trace.KeyValue.newBuilder() .setVStr(message) diff --git a/astra/src/test/java/com/slack/astra/testlib/TemporaryLogStoreAndSearcherExtension.java b/astra/src/test/java/com/slack/astra/testlib/TemporaryLogStoreAndSearcherExtension.java index 7d50e10fc8..0c9ba28c0b 100644 --- a/astra/src/test/java/com/slack/astra/testlib/TemporaryLogStoreAndSearcherExtension.java +++ b/astra/src/test/java/com/slack/astra/testlib/TemporaryLogStoreAndSearcherExtension.java @@ -24,6 +24,7 @@ import org.apache.commons.io.FileUtils; import org.junit.jupiter.api.extension.AfterEachCallback; import org.junit.jupiter.api.extension.ExtensionContext; +import org.opensearch.index.query.QueryBuilder; public class TemporaryLogStoreAndSearcherExtension implements AfterEachCallback { @@ -43,6 +44,15 @@ public static void addMessages( public static List findAllMessages( LogIndexSearcherImpl searcher, String dataset, String query, int howMany) { + return findAllMessages(searcher, dataset, query, howMany, null); + } + + public static List findAllMessages( + LogIndexSearcherImpl searcher, + String dataset, + String query, + int howMany, + QueryBuilder queryBuilder) { SearchResult results = searcher.search( dataset, @@ -51,7 +61,8 @@ public static List findAllMessages( MAX_TIME, howMany, new DateHistogramAggBuilder( - "1", LogMessage.SystemField.TIME_SINCE_EPOCH.fieldName, "1s")); + "1", LogMessage.SystemField.TIME_SINCE_EPOCH.fieldName, "1s"), + queryBuilder); return results.hits; } diff --git a/astra/src/test/java/com/slack/astra/writer/LogMessageWriterImplTest.java b/astra/src/test/java/com/slack/astra/writer/LogMessageWriterImplTest.java index 9933809d9f..e0d3ee477c 100644 --- a/astra/src/test/java/com/slack/astra/writer/LogMessageWriterImplTest.java +++ b/astra/src/test/java/com/slack/astra/writer/LogMessageWriterImplTest.java @@ -96,7 +96,8 @@ private SearchResult searchChunkManager(String indexName, String que 10, new DateHistogramAggBuilder( "1", LogMessage.SystemField.TIME_SINCE_EPOCH.fieldName, "1s"), - Collections.emptyList()), + Collections.emptyList(), + null), Duration.ofMillis(3000)); } @@ -178,7 +179,8 @@ public void testAvgMessageSizeCalculationOnSpanIngestion() throws Exception { 100, new DateHistogramAggBuilder( "1", LogMessage.SystemField.TIME_SINCE_EPOCH.fieldName, "1s"), - Collections.emptyList()), + Collections.emptyList(), + null), Duration.ofMillis(3000)) .hits .size()) From 2312fdf7fa12663bd8cd3211717d00a18fa90816 Mon Sep 17 00:00:00 2001 From: kyle-sammons Date: Fri, 12 Jul 2024 11:25:26 -0700 Subject: [PATCH 2/6] Formatting --- .../com/slack/astra/logstore/opensearch/OpenSearchAdapter.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/astra/src/main/java/com/slack/astra/logstore/opensearch/OpenSearchAdapter.java b/astra/src/main/java/com/slack/astra/logstore/opensearch/OpenSearchAdapter.java index 8e5c4c8585..60d7c61bfb 100644 --- a/astra/src/main/java/com/slack/astra/logstore/opensearch/OpenSearchAdapter.java +++ b/astra/src/main/java/com/slack/astra/logstore/opensearch/OpenSearchAdapter.java @@ -143,7 +143,7 @@ public OpenSearchAdapter(Map chunkSchema) { this.mapperService = buildMapperService(indexSettings, similarityService); this.chunkSchema = chunkSchema; this.useOpenSearchQueryParsing = - Boolean.parseBoolean(System.getProperty("astra.bulkIngest.useKafkaTransactions", "false")); + Boolean.parseBoolean(System.getProperty("astra.bulkIngest.useKafkaTransactions", "false")); } /** From 06b217e83f19dad50213279a332188457ef7586e Mon Sep 17 00:00:00 2001 From: kyle-sammons Date: Fri, 12 Jul 2024 12:04:07 -0700 Subject: [PATCH 3/6] Fix property name and test --- .../com/slack/astra/logstore/opensearch/OpenSearchAdapter.java | 2 +- .../slack/astra/logstore/opensearch/OpenSearchAdapterTest.java | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/astra/src/main/java/com/slack/astra/logstore/opensearch/OpenSearchAdapter.java b/astra/src/main/java/com/slack/astra/logstore/opensearch/OpenSearchAdapter.java index 60d7c61bfb..8806cb57bd 100644 --- a/astra/src/main/java/com/slack/astra/logstore/opensearch/OpenSearchAdapter.java +++ b/astra/src/main/java/com/slack/astra/logstore/opensearch/OpenSearchAdapter.java @@ -143,7 +143,7 @@ public OpenSearchAdapter(Map chunkSchema) { this.mapperService = buildMapperService(indexSettings, similarityService); this.chunkSchema = chunkSchema; this.useOpenSearchQueryParsing = - Boolean.parseBoolean(System.getProperty("astra.bulkIngest.useKafkaTransactions", "false")); + Boolean.parseBoolean(System.getProperty("astra.query.useOpenSearchParsing", "false")); } /** diff --git a/astra/src/test/java/com/slack/astra/logstore/opensearch/OpenSearchAdapterTest.java b/astra/src/test/java/com/slack/astra/logstore/opensearch/OpenSearchAdapterTest.java index 1b6d6fae77..aafe09630c 100644 --- a/astra/src/test/java/com/slack/astra/logstore/opensearch/OpenSearchAdapterTest.java +++ b/astra/src/test/java/com/slack/astra/logstore/opensearch/OpenSearchAdapterTest.java @@ -80,6 +80,7 @@ public OpenSearchAdapterTest() throws IOException { true, true)); openSearchAdapter = new OpenSearchAdapter(fieldDefBuilder.build()); + System.setProperty("astra.query.useOpenSearchParsing", "true"); // We need to reload the schema so that query optimizations take into account the schema openSearchAdapter.reloadSchema(); From 426873c6d53b11189047551010b295ff746df185 Mon Sep 17 00:00:00 2001 From: kyle-sammons Date: Mon, 15 Jul 2024 10:50:26 -0700 Subject: [PATCH 4/6] PR comments --- .../slack/astra/logstore/opensearch/OpenSearchAdapter.java | 6 ++---- .../com/slack/astra/logstore/search/SearchResultUtils.java | 2 +- .../astra/logstore/opensearch/OpenSearchAdapterTest.java | 4 ++-- 3 files changed, 5 insertions(+), 7 deletions(-) diff --git a/astra/src/main/java/com/slack/astra/logstore/opensearch/OpenSearchAdapter.java b/astra/src/main/java/com/slack/astra/logstore/opensearch/OpenSearchAdapter.java index 8806cb57bd..3b2c39f297 100644 --- a/astra/src/main/java/com/slack/astra/logstore/opensearch/OpenSearchAdapter.java +++ b/astra/src/main/java/com/slack/astra/logstore/opensearch/OpenSearchAdapter.java @@ -135,7 +135,7 @@ public class OpenSearchAdapter { // This will enable OpenSearch query parsing by default, rather than going down the // QueryString parsing path we have been using - private boolean useOpenSearchQueryParsing = false; + private final boolean useOpenSearchQueryParsing; public OpenSearchAdapter(Map chunkSchema) { this.indexSettings = buildIndexSettings(); @@ -421,9 +421,7 @@ protected static IndexSettings buildIndexSettings() { Settings nodeSetings = Settings.builder().put("indices.query.query_string.analyze_wildcard", true).build(); - Set> built = new HashSet<>(BUILT_IN_INDEX_SETTINGS); - - IndexScopedSettings indexScopedSettings = new IndexScopedSettings(settings, built); + IndexScopedSettings indexScopedSettings = new IndexScopedSettings(settings, new HashSet<>(BUILT_IN_INDEX_SETTINGS)); return new IndexSettings( IndexMetadata.builder("index").settings(settings).build(), diff --git a/astra/src/main/java/com/slack/astra/logstore/search/SearchResultUtils.java b/astra/src/main/java/com/slack/astra/logstore/search/SearchResultUtils.java index c59cb3f1c9..686b9fe5a5 100644 --- a/astra/src/main/java/com/slack/astra/logstore/search/SearchResultUtils.java +++ b/astra/src/main/java/com/slack/astra/logstore/search/SearchResultUtils.java @@ -679,7 +679,7 @@ public static SearchQuery fromSearchRequest(AstraSearch.SearchRequest searchRequ objectMapper.createParser(searchRequest.getQuery())); queryBuilder = AbstractQueryBuilder.parseInnerQueryBuilder(jsonXContentParser); } catch (Exception e) { - throw new RuntimeException(e); + throw new IllegalArgumentException(e); } } diff --git a/astra/src/test/java/com/slack/astra/logstore/opensearch/OpenSearchAdapterTest.java b/astra/src/test/java/com/slack/astra/logstore/opensearch/OpenSearchAdapterTest.java index aafe09630c..26278a812b 100644 --- a/astra/src/test/java/com/slack/astra/logstore/opensearch/OpenSearchAdapterTest.java +++ b/astra/src/test/java/com/slack/astra/logstore/opensearch/OpenSearchAdapterTest.java @@ -80,8 +80,6 @@ public OpenSearchAdapterTest() throws IOException { true, true)); openSearchAdapter = new OpenSearchAdapter(fieldDefBuilder.build()); - System.setProperty("astra.query.useOpenSearchParsing", "true"); - // We need to reload the schema so that query optimizations take into account the schema openSearchAdapter.reloadSchema(); } @@ -457,6 +455,7 @@ public void handlesDateHistogramExtendedBoundsMinDocEdgeCases() throws IOExcepti @Test public void shouldProduceQueryFromQueryBuilder() throws Exception { + System.setProperty("astra.query.useOpenSearchParsing", "true"); BoolQueryBuilder boolQueryBuilder = new BoolQueryBuilder().filter(new RangeQueryBuilder("_timesinceepoch").gte(1).lte(100)); IndexSearcher indexSearcher = logStoreAndSearcherRule.logStore.getSearcherManager().acquire(); @@ -464,6 +463,7 @@ public void shouldProduceQueryFromQueryBuilder() throws Exception { openSearchAdapter.buildQuery("foo", null, null, null, indexSearcher, boolQueryBuilder); assertThat(rangeQuery).isNotNull(); assertThat(rangeQuery.toString()).isEqualTo("#_timesinceepoch:[1 TO 100]"); + System.setProperty("astra.query.useOpenSearchParsing", "false"); } @Test From 57e7b725d5a866465e2db7b7f31308720d0abf2b Mon Sep 17 00:00:00 2001 From: kyle-sammons Date: Mon, 15 Jul 2024 10:54:01 -0700 Subject: [PATCH 5/6] Formatting --- .../slack/astra/logstore/opensearch/OpenSearchAdapter.java | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/astra/src/main/java/com/slack/astra/logstore/opensearch/OpenSearchAdapter.java b/astra/src/main/java/com/slack/astra/logstore/opensearch/OpenSearchAdapter.java index 3b2c39f297..d597e6becc 100644 --- a/astra/src/main/java/com/slack/astra/logstore/opensearch/OpenSearchAdapter.java +++ b/astra/src/main/java/com/slack/astra/logstore/opensearch/OpenSearchAdapter.java @@ -34,7 +34,6 @@ import java.util.HashSet; import java.util.List; import java.util.Map; -import java.util.Set; import java.util.TreeMap; import java.util.stream.Collectors; import org.apache.commons.lang3.ObjectUtils; @@ -48,7 +47,6 @@ import org.opensearch.common.CheckedConsumer; import org.opensearch.common.compress.CompressedXContent; import org.opensearch.common.settings.IndexScopedSettings; -import org.opensearch.common.settings.Setting; import org.opensearch.common.settings.Settings; import org.opensearch.common.util.BigArrays; import org.opensearch.common.xcontent.XContentFactory; @@ -421,7 +419,8 @@ protected static IndexSettings buildIndexSettings() { Settings nodeSetings = Settings.builder().put("indices.query.query_string.analyze_wildcard", true).build(); - IndexScopedSettings indexScopedSettings = new IndexScopedSettings(settings, new HashSet<>(BUILT_IN_INDEX_SETTINGS)); + IndexScopedSettings indexScopedSettings = + new IndexScopedSettings(settings, new HashSet<>(BUILT_IN_INDEX_SETTINGS)); return new IndexSettings( IndexMetadata.builder("index").settings(settings).build(), From 54bafa6658fe0d0811c4e2b210be744590613344 Mon Sep 17 00:00:00 2001 From: kyle-sammons Date: Mon, 15 Jul 2024 11:11:08 -0700 Subject: [PATCH 6/6] Fix test by recreating object --- .../opensearch/OpenSearchAdapterTest.java | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) diff --git a/astra/src/test/java/com/slack/astra/logstore/opensearch/OpenSearchAdapterTest.java b/astra/src/test/java/com/slack/astra/logstore/opensearch/OpenSearchAdapterTest.java index 26278a812b..0e8060cd81 100644 --- a/astra/src/test/java/com/slack/astra/logstore/opensearch/OpenSearchAdapterTest.java +++ b/astra/src/test/java/com/slack/astra/logstore/opensearch/OpenSearchAdapterTest.java @@ -63,10 +63,11 @@ public class OpenSearchAdapterTest { public TemporaryLogStoreAndSearcherExtension logStoreAndSearcherRule = new TemporaryLogStoreAndSearcherExtension(false); + private final ImmutableMap.Builder fieldDefBuilder; private final OpenSearchAdapter openSearchAdapter; public OpenSearchAdapterTest() throws IOException { - ImmutableMap.Builder fieldDefBuilder = ImmutableMap.builder(); + fieldDefBuilder = ImmutableMap.builder(); fieldDefBuilder.put( LogMessage.SystemField.ID.fieldName, new LuceneFieldDef( @@ -455,15 +456,23 @@ public void handlesDateHistogramExtendedBoundsMinDocEdgeCases() throws IOExcepti @Test public void shouldProduceQueryFromQueryBuilder() throws Exception { - System.setProperty("astra.query.useOpenSearchParsing", "true"); BoolQueryBuilder boolQueryBuilder = new BoolQueryBuilder().filter(new RangeQueryBuilder("_timesinceepoch").gte(1).lte(100)); IndexSearcher indexSearcher = logStoreAndSearcherRule.logStore.getSearcherManager().acquire(); + + // We need to recreate the OpenSearchAdapter object here to get the feature flag set to true. + // Once this feature flag is removed, we can once again use the class-level object + System.setProperty("astra.query.useOpenSearchParsing", "true"); + OpenSearchAdapter openSearchAdapterWithFeatureFlagEnabled = + new OpenSearchAdapter(fieldDefBuilder.build()); + openSearchAdapterWithFeatureFlagEnabled.reloadSchema(); + System.setProperty("astra.query.useOpenSearchParsing", "false"); + Query rangeQuery = - openSearchAdapter.buildQuery("foo", null, null, null, indexSearcher, boolQueryBuilder); + openSearchAdapterWithFeatureFlagEnabled.buildQuery( + "foo", null, null, null, indexSearcher, boolQueryBuilder); assertThat(rangeQuery).isNotNull(); assertThat(rangeQuery.toString()).isEqualTo("#_timesinceepoch:[1 TO 100]"); - System.setProperty("astra.query.useOpenSearchParsing", "false"); } @Test