From 211038d81fac9776c8e7be04ba8becea4ca4a59a Mon Sep 17 00:00:00 2001 From: saxenakshitiz Date: Tue, 21 May 2024 12:35:57 +0530 Subject: [PATCH 1/6] chore: use filter in entities query in selections as well --- .../service/common/ExpressionContext.java | 49 +++ .../query/visitor/ExecutionVisitor.java | 16 +- .../service/common/AbstractServiceTest.java | 11 +- .../explore/entity/EntityServiceTest.java | 60 ++++ .../test/resources/attributes/attributes.json | 33 ++ .../entity/backend-distinct-count.json | 94 +++++ .../entity/backend-distinct-count.json | 332 ++++++++++++++++++ .../entity/backend-distinct-count.json | 81 +++++ 8 files changed, 672 insertions(+), 4 deletions(-) create mode 100644 gateway-service-impl/src/test/java/org/hypertrace/gateway/service/explore/entity/EntityServiceTest.java create mode 100644 gateway-service-impl/src/test/resources/expected-test-responses/entity/backend-distinct-count.json create mode 100644 gateway-service-impl/src/test/resources/query-service-requests-and-responses/entity/backend-distinct-count.json create mode 100644 gateway-service-impl/src/test/resources/test-requests/entity/backend-distinct-count.json diff --git a/gateway-service-impl/src/main/java/org/hypertrace/gateway/service/common/ExpressionContext.java b/gateway-service-impl/src/main/java/org/hypertrace/gateway/service/common/ExpressionContext.java index f13a9cf8..97c9032a 100644 --- a/gateway-service-impl/src/main/java/org/hypertrace/gateway/service/common/ExpressionContext.java +++ b/gateway-service-impl/src/main/java/org/hypertrace/gateway/service/common/ExpressionContext.java @@ -1,6 +1,8 @@ package org.hypertrace.gateway.service.common; +import static java.util.Collections.emptyMap; import static java.util.function.Predicate.not; +import static org.hypertrace.core.attribute.service.v1.AttributeSource.QS; import com.google.common.collect.ImmutableMap; import com.google.common.collect.Sets; @@ -53,6 +55,8 @@ public class ExpressionContext { private ImmutableMap> sourceToOrderByExpressionMap; + private Map sourceToAndFilterMap; + // filters private final Filter filter; private ImmutableMap> sourceToFilterExpressionMap; @@ -91,6 +95,8 @@ public ExpressionContext( buildSourceToOrderByExpressionMaps(); buildSourceToGroupByExpressionMaps(); + this.sourceToAndFilterMap = buildSourceToAndFilterMap(filter); + this.isAndFilter = gatewayServiceConfig.isEntityAndFilterEnabled() && isAndFilter(filter); } @@ -106,6 +112,10 @@ public void setSourceToSelectionExpressionMap( .build(); } + public Map getSourceToAndFilterMap() { + return sourceToAndFilterMap; + } + public Map> getSourceToSelectionAttributeMap() { return sourceToSelectionAttributeMap; } @@ -620,6 +630,45 @@ private static Set getIntersectingSourceSets( .orElse(Collections.emptySet()); } + private Map buildSourceToAndFilterMap(Filter filter) { + Operator operator = filter.getOperator(); + if (operator == Operator.AND) { + return filter.getChildFilterList().stream() + .map(this::buildSourceToAndFilterMap) + .flatMap(map -> map.entrySet().stream()) + .collect( + Collectors.toUnmodifiableMap( + Map.Entry::getKey, + Map.Entry::getValue, + (value1, value2) -> + Filter.newBuilder() + .setOperator(Operator.AND) + .addChildFilter(value1) + .addChildFilter(value2) + .build())); + + } else if (operator == Operator.OR) { + return Collections.emptyMap(); + } else { + List attributeSources = getAttributeSources(filter.getLhs()); + if (attributeSources.isEmpty()) { + return emptyMap(); + } + + return attributeSources.contains(QS) + ? Map.of(QS.name(), filter) + : Map.of(attributeSources.get(0).name(), filter); + } + } + + private List getAttributeSources(Expression expression) { + Set attributeIds = ExpressionReader.extractAttributeIds(expression); + return attributeIds.stream() + .map(attributeId -> attributeMetadataMap.get(attributeId).getSourcesList()) + .flatMap(Collection::stream) + .collect(Collectors.toUnmodifiableList()); + } + @Override public String toString() { return "ExpressionContext{" diff --git a/gateway-service-impl/src/main/java/org/hypertrace/gateway/service/entity/query/visitor/ExecutionVisitor.java b/gateway-service-impl/src/main/java/org/hypertrace/gateway/service/entity/query/visitor/ExecutionVisitor.java index f5e372ea..206ee2b5 100644 --- a/gateway-service-impl/src/main/java/org/hypertrace/gateway/service/entity/query/visitor/ExecutionVisitor.java +++ b/gateway-service-impl/src/main/java/org/hypertrace/gateway/service/entity/query/visitor/ExecutionVisitor.java @@ -295,7 +295,7 @@ public EntityResponse visit(SelectionNode selectionNode) { .getExpressionContext() .getSourceToMetricExpressionMap() .get(source)) - .setFilter(filter) + .setFilter(addSourceFilters(executionContext, source, filter)) .build(); IEntityFetcher entityFetcher = queryHandlerRegistry.getEntityFetcher(source); EntitiesRequestContext context = @@ -355,6 +355,20 @@ public EntityResponse visit(SelectionNode selectionNode) { } } + private Filter addSourceFilters( + EntityExecutionContext executionContext, String source, Filter filter) { + Filter andChildFilter = + executionContext.getExpressionContext().getSourceToAndFilterMap().get(source); + if (andChildFilter == null) { + return filter; + } + return Filter.newBuilder() + .setOperator(Operator.AND) + .addChildFilter(filter) + .addAllChildFilter(andChildFilter.getChildFilterList()) + .build(); + } + Filter constructFilterFromChildNodesResult(EntityFetcherResponse result) { if (result.isEmpty()) { return Filter.getDefaultInstance(); diff --git a/gateway-service-impl/src/test/java/org/hypertrace/gateway/service/common/AbstractServiceTest.java b/gateway-service-impl/src/test/java/org/hypertrace/gateway/service/common/AbstractServiceTest.java index 34d93eeb..81d52e38 100644 --- a/gateway-service-impl/src/test/java/org/hypertrace/gateway/service/common/AbstractServiceTest.java +++ b/gateway-service-impl/src/test/java/org/hypertrace/gateway/service/common/AbstractServiceTest.java @@ -19,7 +19,6 @@ import java.io.Reader; import java.util.ArrayList; import java.util.Arrays; -import java.util.Collections; import java.util.HashMap; import java.util.HashSet; import java.util.Iterator; @@ -39,6 +38,7 @@ import org.hypertrace.gateway.service.common.config.ScopeFilterConfigs; import org.hypertrace.gateway.service.common.util.QueryServiceClient; import org.hypertrace.gateway.service.entity.config.EntityIdColumnsConfig; +import org.hypertrace.gateway.service.entity.config.LogConfig; import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.params.ParameterizedTest; @@ -82,14 +82,19 @@ public static void setUp() throws IOException { + " }\n" + " ]\n" + " }\n" - + "]"; + + "]\n" + + "entity.service.log.config = {\n" + + " query.threshold.millis = 1500\n" + + "}\n"; Config config = ConfigFactory.parseString(scopeFiltersConfig); scopeFilterConfigs = new ScopeFilterConfigs(config); - entityIdColumnsConfig = new EntityIdColumnsConfig(Collections.emptyMap()); + entityIdColumnsConfig = new EntityIdColumnsConfig(Map.of("BACKEND", "id")); gatewayServiceConfig = mock(GatewayServiceConfig.class); when(gatewayServiceConfig.getEntityIdColumnsConfig()).thenReturn(entityIdColumnsConfig); when(gatewayServiceConfig.getScopeFilterConfigs()).thenReturn(scopeFilterConfigs); entityTypesProvider = mock(EntityTypesProvider.class); + LogConfig logConfig = new LogConfig(config); + when(gatewayServiceConfig.getLogConfig()).thenReturn(logConfig); } private static Reader readResourceFile(String fileName) { diff --git a/gateway-service-impl/src/test/java/org/hypertrace/gateway/service/explore/entity/EntityServiceTest.java b/gateway-service-impl/src/test/java/org/hypertrace/gateway/service/explore/entity/EntityServiceTest.java new file mode 100644 index 00000000..5250c0fe --- /dev/null +++ b/gateway-service-impl/src/test/java/org/hypertrace/gateway/service/explore/entity/EntityServiceTest.java @@ -0,0 +1,60 @@ +package org.hypertrace.gateway.service.explore.entity; + +import com.google.protobuf.GeneratedMessageV3; +import java.util.concurrent.Executors; +import java.util.stream.Stream; +import org.hypertrace.entity.query.service.client.EntityQueryServiceClient; +import org.hypertrace.gateway.service.EntityTypesProvider; +import org.hypertrace.gateway.service.common.AbstractServiceTest; +import org.hypertrace.gateway.service.common.AttributeMetadataProvider; +import org.hypertrace.gateway.service.common.RequestContext; +import org.hypertrace.gateway.service.common.config.GatewayServiceConfig; +import org.hypertrace.gateway.service.common.util.QueryServiceClient; +import org.hypertrace.gateway.service.entity.EntityService; +import org.hypertrace.gateway.service.v1.entity.EntitiesRequest; +import org.hypertrace.gateway.service.v1.entity.EntitiesResponse; + +public class EntityServiceTest extends AbstractServiceTest { + private static final String SUITE_NAME = "entity"; + + public static Stream data() { + return getTestFileNames(SUITE_NAME); + } + + @Override + protected String getTestSuiteName() { + return SUITE_NAME; + } + + @Override + protected GeneratedMessageV3.Builder getGatewayServiceRequestBuilder() { + return EntitiesRequest.newBuilder(); + } + + @Override + protected GeneratedMessageV3.Builder getGatewayServiceResponseBuilder() { + return EntitiesResponse.newBuilder(); + } + + @Override + protected EntitiesResponse executeApi( + GatewayServiceConfig gatewayServiceConfig, + EntitiesRequest request, + QueryServiceClient queryServiceClient, + EntityQueryServiceClient entityQueryServiceClient, + AttributeMetadataProvider attributeMetadataProvider, + EntityTypesProvider entityTypesProvider) { + EntityService entityService = + new EntityService( + gatewayServiceConfig, + queryServiceClient, + entityQueryServiceClient, + null, + attributeMetadataProvider, + Executors.newFixedThreadPool(1)); + return entityService.getEntities( + new RequestContext( + org.hypertrace.core.grpcutils.context.RequestContext.forTenantId(TENANT_ID)), + request); + } +} diff --git a/gateway-service-impl/src/test/resources/attributes/attributes.json b/gateway-service-impl/src/test/resources/attributes/attributes.json index 48c3665c..0d1177ac 100644 --- a/gateway-service-impl/src/test/resources/attributes/attributes.json +++ b/gateway-service-impl/src/test/resources/attributes/attributes.json @@ -1312,6 +1312,39 @@ "QS" ] }, + { + "fqn": "Backend.type", + "valueKind": "TYPE_STRING", + "key": "type", + "displayName": "Type", + "scopeString": "BACKEND", + "type": "ATTRIBUTE", + "sources": [ + "QS" + ] + }, + { + "fqn": "Backend.backendApiId", + "valueKind": "TYPE_STRING", + "key": "backendApiId", + "displayName": "BackendApiId", + "scopeString": "BACKEND", + "type": "ATTRIBUTE", + "sources": [ + "QS" + ] + }, + { + "fqn": "Backend.environment", + "valueKind": "TYPE_STRING", + "key": "environment", + "displayName": "Environment", + "scopeString": "BACKEND", + "type": "ATTRIBUTE", + "sources": [ + "QS" + ] + }, { "fqn": "Backend.metrics.bytes_received", "valueKind": "TYPE_INT64", diff --git a/gateway-service-impl/src/test/resources/expected-test-responses/entity/backend-distinct-count.json b/gateway-service-impl/src/test/resources/expected-test-responses/entity/backend-distinct-count.json new file mode 100644 index 00000000..bdec9237 --- /dev/null +++ b/gateway-service-impl/src/test/resources/expected-test-responses/entity/backend-distinct-count.json @@ -0,0 +1,94 @@ +{ + "entity": [{ + "id": "backend-id-1", + "entityType": "BACKEND", + "attribute": { + "BACKEND.id": { + "valueType": "STRING", + "string": "backend-id-1" + }, + "id": { + "valueType": "STRING", + "string": "backend-id-1" + }, + "type": { + "valueType": "STRING", + "string": "HTTPS" + }, + "name": { + "valueType": "STRING", + "string": "backend-1.abc.com" + } + }, + "metric": { + "DISTINCT_COUNT_BACKEND.backendApiId_[]": { + "function": "DISTINCTCOUNT", + "value": { + "valueType": "LONG", + "long": "2" + } + } + } + }, { + "id": "backend-id-2", + "entityType": "BACKEND", + "attribute": { + "BACKEND.id": { + "valueType": "STRING", + "string": "backend-id-2" + }, + "id": { + "valueType": "STRING", + "string": "backend-id-2" + }, + "type": { + "valueType": "STRING", + "string": "HTTPS" + }, + "name": { + "valueType": "STRING", + "string": "backend-2.abc.com" + } + }, + "metric": { + "DISTINCT_COUNT_BACKEND.backendApiId_[]": { + "function": "DISTINCTCOUNT", + "value": { + "valueType": "LONG", + "long": "2" + } + } + } + }, { + "id": "backend-id-3", + "entityType": "BACKEND", + "attribute": { + "BACKEND.id": { + "valueType": "STRING", + "string": "backend-id-3" + }, + "id": { + "valueType": "STRING", + "string": "backend-id-3" + }, + "type": { + "valueType": "STRING", + "string": "HTTPS" + }, + "name": { + "valueType": "STRING", + "string": "backend-3.abc.com" + } + }, + "metric": { + "DISTINCT_COUNT_BACKEND.backendApiId_[]": { + "function": "DISTINCTCOUNT", + "value": { + "valueType": "LONG", + "long": "2" + } + } + } + }], + "total": 3 +} \ No newline at end of file diff --git a/gateway-service-impl/src/test/resources/query-service-requests-and-responses/entity/backend-distinct-count.json b/gateway-service-impl/src/test/resources/query-service-requests-and-responses/entity/backend-distinct-count.json new file mode 100644 index 00000000..0182f968 --- /dev/null +++ b/gateway-service-impl/src/test/resources/query-service-requests-and-responses/entity/backend-distinct-count.json @@ -0,0 +1,332 @@ +[ + { + "request": { + "filter": { + "childFilter": [{ + "lhs": { + "attributeExpression": { + "attributeId": "BACKEND.id" + } + }, + "operator": "NEQ", + "rhs": { + "literal": { + "value": { + "valueType": "NULL_STRING" + } + } + } + }, { + "childFilter": [{ + "lhs": { + "attributeExpression": { + "attributeId": "BACKEND.startTime" + } + }, + "operator": "GE", + "rhs": { + "literal": { + "value": { + "valueType": "LONG", + "long": "1715779687497" + } + } + } + }, { + "lhs": { + "attributeExpression": { + "attributeId": "BACKEND.startTime" + } + }, + "operator": "LT", + "rhs": { + "literal": { + "value": { + "valueType": "LONG", + "long": "1715866087497" + } + } + } + }] + }, { + "childFilter": [{ + "lhs": { + "attributeExpression": { + "attributeId": "BACKEND.environment", + "alias": "environment" + } + }, + "operator": "EQ", + "rhs": { + "literal": { + "value": { + "string": "testenv" + } + } + } + }, { + "lhs": { + "attributeExpression": { + "attributeId": "BACKEND.backendApiId", + "alias": "backendApiId" + } + }, + "operator": "NEQ", + "rhs": { + "literal": { + "value": { + "string": "null" + } + } + } + }] + }] + }, + "selection": [{ + "attributeExpression": { + "attributeId": "BACKEND.id" + } + }, { + "attributeExpression": { + "attributeId": "BACKEND.type", + "alias": "type" + } + }, { + "attributeExpression": { + "attributeId": "BACKEND.name", + "alias": "name" + } + }, { + "function": { + "functionName": "COUNT", + "arguments": [{ + "attributeExpression": { + "attributeId": "BACKEND.id" + } + }] + } + }], + "groupBy": [{ + "attributeExpression": { + "attributeId": "BACKEND.id" + } + }, { + "attributeExpression": { + "attributeId": "BACKEND.type", + "alias": "type" + } + }, { + "attributeExpression": { + "attributeId": "BACKEND.name", + "alias": "name" + } + }], + "orderBy": [{ + "expression": { + "attributeExpression": { + "attributeId": "BACKEND.name", + "alias": "name" + } + } + }], + "limit": 10000 + }, + "response": { + "isLastChunk": true, + "resultSetMetadata": { + "columnMetadata": [{ + "columnName": "BACKEND.id" + }, { + "columnName": "type" + }, { + "columnName": "name" + }, { + "columnName": "COUNT" + }] + }, + "row": [{ + "column": [{ + "string": "backend-id-1" + }, { + "string": "HTTPS" + }, { + "string": "backend-1.abc.com" + }, { + "string": "15746" + }] + }, { + "column": [{ + "string": "backend-id-2" + }, { + "string": "HTTPS" + }, { + "string": "backend-2.abc.com" + }, { + "string": "26190" + }] + }, { + "column": [{ + "string": "backend-id-3" + }, { + "string": "HTTPS" + }, { + "string": "backend-3.abc.com" + }, { + "string": "31634" + }] + }] + } + }, + { + "request": { + "filter": { + "childFilter": [{ + "lhs": { + "attributeExpression": { + "attributeId": "BACKEND.id" + } + }, + "operator": "NEQ", + "rhs": { + "literal": { + "value": { + "valueType": "NULL_STRING" + } + } + } + }, { + "childFilter": [{ + "lhs": { + "attributeExpression": { + "attributeId": "BACKEND.startTime" + } + }, + "operator": "GE", + "rhs": { + "literal": { + "value": { + "valueType": "LONG", + "long": "1715779687497" + } + } + } + }, { + "lhs": { + "attributeExpression": { + "attributeId": "BACKEND.startTime" + } + }, + "operator": "LT", + "rhs": { + "literal": { + "value": { + "valueType": "LONG", + "long": "1715866087497" + } + } + } + }] + }, { + "childFilter": [{ + "lhs": { + "attributeExpression": { + "attributeId": "BACKEND.id", + "alias": "entityId0" + } + }, + "operator": "IN", + "rhs": { + "literal": { + "value": { + "valueType": "STRING_ARRAY", + "stringArray": ["backend-id-3", "backend-id-1", "backend-id-2"] + } + } + } + }, { + "lhs": { + "attributeExpression": { + "attributeId": "BACKEND.environment", + "alias": "environment" + } + }, + "operator": "EQ", + "rhs": { + "literal": { + "value": { + "string": "testenv" + } + } + } + }, { + "lhs": { + "attributeExpression": { + "attributeId": "BACKEND.backendApiId", + "alias": "backendApiId" + } + }, + "operator": "NEQ", + "rhs": { + "literal": { + "value": { + "string": "null" + } + } + } + }] + }] + }, + "selection": [{ + "attributeExpression": { + "attributeId": "BACKEND.id" + } + }, { + "function": { + "functionName": "DISTINCTCOUNT", + "arguments": [{ + "attributeExpression": { + "attributeId": "BACKEND.backendApiId", + "alias": "backendApiId" + } + }], + "alias": "DISTINCT_COUNT_BACKEND.backendApiId_[]" + } + }], + "groupBy": [{ + "attributeExpression": { + "attributeId": "BACKEND.id" + } + }], + "limit": 10000 + }, + "response": { + "isLastChunk": true, + "resultSetMetadata": { + "columnMetadata": [{ + "columnName": "BACKEND.id" + }, { + "columnName": "DISTINCT_COUNT_BACKEND.backendApiId_[]" + }] + }, + "row": [{ + "column": [{ + "string": "backend-id-3" + }, { + "string": "2" + }] + }, { + "column": [{ + "string": "backend-id-2" + }, { + "string": "2" + }] + }, { + "column": [{ + "string": "backend-id-1" + }, { + "string": "2" + }] + }] + } + } +] \ No newline at end of file diff --git a/gateway-service-impl/src/test/resources/test-requests/entity/backend-distinct-count.json b/gateway-service-impl/src/test/resources/test-requests/entity/backend-distinct-count.json new file mode 100644 index 00000000..3d8acb67 --- /dev/null +++ b/gateway-service-impl/src/test/resources/test-requests/entity/backend-distinct-count.json @@ -0,0 +1,81 @@ +{ + "entityType": "BACKEND", + "startTimeMillis": "1715779687497", + "endTimeMillis": "1715866087497", + "filter": { + "operator": "AND", + "childFilter": [{ + "lhs": { + "attributeExpression": { + "attributeId": "BACKEND.environment", + "alias": "environment" + } + }, + "operator": "EQ", + "rhs": { + "literal": { + "value": { + "valueType": "STRING", + "string": "testenv" + } + } + } + }, { + "lhs": { + "attributeExpression": { + "attributeId": "BACKEND.backendApiId", + "alias": "backendApiId" + } + }, + "operator": "NEQ", + "rhs": { + "literal": { + "value": { + "valueType": "STRING", + "string": "null" + } + } + } + }] + }, + "selection": [{ + "attributeExpression": { + "attributeId": "BACKEND.type", + "alias": "type" + } + }, { + "attributeExpression": { + "attributeId": "BACKEND.id", + "alias": "id" + } + }, { + "attributeExpression": { + "attributeId": "BACKEND.name", + "alias": "name" + } + }, { + "function": { + "function": "DISTINCTCOUNT", + "arguments": [{ + "attributeExpression": { + "attributeId": "BACKEND.backendApiId", + "alias": "backendApiId" + } + }], + "alias": "DISTINCT_COUNT_BACKEND.backendApiId_[]" + } + }], + "incomingInteractions": { + }, + "outgoingInteractions": { + }, + "orderBy": [{ + "expression": { + "attributeExpression": { + "attributeId": "BACKEND.name", + "alias": "name" + } + } + }], + "limit": 50 +} \ No newline at end of file From 9a97d111d3c64b0065ca324b376cddc5bca1d01a Mon Sep 17 00:00:00 2001 From: saxenakshitiz Date: Tue, 21 May 2024 13:34:25 +0530 Subject: [PATCH 2/6] fix test failure --- .../EntityServiceAndGatewayServiceConverterTest.java | 6 +++--- .../test/resources/configs/gateway-service/application.conf | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/gateway-service-impl/src/test/java/org/hypertrace/gateway/service/common/converter/EntityServiceAndGatewayServiceConverterTest.java b/gateway-service-impl/src/test/java/org/hypertrace/gateway/service/common/converter/EntityServiceAndGatewayServiceConverterTest.java index b990a9c6..8e0dcfff 100644 --- a/gateway-service-impl/src/test/java/org/hypertrace/gateway/service/common/converter/EntityServiceAndGatewayServiceConverterTest.java +++ b/gateway-service-impl/src/test/java/org/hypertrace/gateway/service/common/converter/EntityServiceAndGatewayServiceConverterTest.java @@ -24,13 +24,13 @@ import org.hypertrace.gateway.service.v1.entity.EntitiesRequest; import org.junit.jupiter.api.Test; -public class EntityServiceAndGatewayServiceConverterTest extends AbstractGatewayServiceTest { +class EntityServiceAndGatewayServiceConverterTest extends AbstractGatewayServiceTest { @Test - public void testAddBetweenFilter() { + void testAddBetweenFilter() { int startTimeMillis = 1; int endTimeMillis = 2; - String timestamp = "lastActivity"; + String timestamp = "startTime"; String timestampAttributeName = BACKEND.name() + "." + timestamp; Expression.Builder expectedStartTimeConstant = diff --git a/gateway-service-impl/src/test/resources/configs/gateway-service/application.conf b/gateway-service-impl/src/test/resources/configs/gateway-service/application.conf index 667bc910..4335956a 100644 --- a/gateway-service-impl/src/test/resources/configs/gateway-service/application.conf +++ b/gateway-service-impl/src/test/resources/configs/gateway-service/application.conf @@ -46,7 +46,7 @@ interaction.config = [ timestamp.config = [ { scope = BACKEND - timestamp = lastActivity + timestamp = startTime }, { scope = LOG_EVENT From 7682c66974d9a6f5b180956632f30b3a1fbf7214 Mon Sep 17 00:00:00 2001 From: saxenakshitiz Date: Thu, 23 May 2024 18:53:43 +0530 Subject: [PATCH 3/6] added filters for selection for time aggregated queries as well --- .../service/common/ExpressionContext.java | 13 ++-- .../entity/query/ExecutionTreeBuilder.java | 3 + .../query/visitor/ExecutionVisitor.java | 27 ++++--- .../query/visitor/SourceFilterVisitor.java | 76 +++++++++++++++++++ .../entity/backend-distinct-count.json | 52 +++++++------ 5 files changed, 129 insertions(+), 42 deletions(-) create mode 100644 gateway-service-impl/src/main/java/org/hypertrace/gateway/service/entity/query/visitor/SourceFilterVisitor.java diff --git a/gateway-service-impl/src/main/java/org/hypertrace/gateway/service/common/ExpressionContext.java b/gateway-service-impl/src/main/java/org/hypertrace/gateway/service/common/ExpressionContext.java index 97c9032a..d7b73fff 100644 --- a/gateway-service-impl/src/main/java/org/hypertrace/gateway/service/common/ExpressionContext.java +++ b/gateway-service-impl/src/main/java/org/hypertrace/gateway/service/common/ExpressionContext.java @@ -55,13 +55,12 @@ public class ExpressionContext { private ImmutableMap> sourceToOrderByExpressionMap; - private Map sourceToAndFilterMap; - // filters private final Filter filter; private ImmutableMap> sourceToFilterExpressionMap; private ImmutableMap> sourceToFilterAttributeMap; private ImmutableMap> filterAttributeToSourceMap; + private Map sourceToFilterMap; // and filter private boolean isAndFilter; @@ -95,7 +94,7 @@ public ExpressionContext( buildSourceToOrderByExpressionMaps(); buildSourceToGroupByExpressionMaps(); - this.sourceToAndFilterMap = buildSourceToAndFilterMap(filter); + this.sourceToFilterMap = new HashMap<>(); this.isAndFilter = gatewayServiceConfig.isEntityAndFilterEnabled() && isAndFilter(filter); } @@ -112,8 +111,12 @@ public void setSourceToSelectionExpressionMap( .build(); } - public Map getSourceToAndFilterMap() { - return sourceToAndFilterMap; + public Map getSourceToFilterMap() { + return sourceToFilterMap; + } + + public void putSourceToFilterMap(String source, Filter filter) { + sourceToFilterMap.put(source, filter); } public Map> getSourceToSelectionAttributeMap() { diff --git a/gateway-service-impl/src/main/java/org/hypertrace/gateway/service/entity/query/ExecutionTreeBuilder.java b/gateway-service-impl/src/main/java/org/hypertrace/gateway/service/entity/query/ExecutionTreeBuilder.java index 23c9dfe5..d8bda4b4 100644 --- a/gateway-service-impl/src/main/java/org/hypertrace/gateway/service/entity/query/ExecutionTreeBuilder.java +++ b/gateway-service-impl/src/main/java/org/hypertrace/gateway/service/entity/query/ExecutionTreeBuilder.java @@ -26,6 +26,7 @@ import org.hypertrace.gateway.service.entity.query.visitor.ExecutionContextBuilderVisitor; import org.hypertrace.gateway.service.entity.query.visitor.FilterOptimizingVisitor; import org.hypertrace.gateway.service.entity.query.visitor.PrintVisitor; +import org.hypertrace.gateway.service.entity.query.visitor.SourceFilterVisitor; import org.hypertrace.gateway.service.v1.common.Expression; import org.hypertrace.gateway.service.v1.common.Filter; import org.hypertrace.gateway.service.v1.common.Operator; @@ -215,6 +216,8 @@ private QueryNode buildExecutionTreeForEdsFilterAndSelection() { @VisibleForTesting QueryNode buildExecutionTree(EntityExecutionContext executionContext, QueryNode filterTree) { QueryNode rootNode = filterTree; + // set up source filter to be used during selections + filterTree.acceptVisitor(new SourceFilterVisitor(executionContext)); // Select attributes from sources in order by but not part of the filter tree Set attrSourcesForOrderBy = executionContext.getPendingSelectionSourcesForOrderBy(); if (!attrSourcesForOrderBy.isEmpty()) { diff --git a/gateway-service-impl/src/main/java/org/hypertrace/gateway/service/entity/query/visitor/ExecutionVisitor.java b/gateway-service-impl/src/main/java/org/hypertrace/gateway/service/entity/query/visitor/ExecutionVisitor.java index 206ee2b5..e7326d4d 100644 --- a/gateway-service-impl/src/main/java/org/hypertrace/gateway/service/entity/query/visitor/ExecutionVisitor.java +++ b/gateway-service-impl/src/main/java/org/hypertrace/gateway/service/entity/query/visitor/ExecutionVisitor.java @@ -10,6 +10,7 @@ import java.util.LinkedList; import java.util.List; import java.util.Map; +import java.util.Optional; import java.util.Set; import java.util.concurrent.CompletableFuture; import java.util.concurrent.ExecutorService; @@ -265,7 +266,7 @@ public EntityResponse visit(SelectionNode selectionNode) { .getExpressionContext() .getSourceToSelectionExpressionMap() .get(source)) - .setFilter(filter) + .setFilter(addSourceFilters(executionContext, source, filter)) .build(); IEntityFetcher entityFetcher = queryHandlerRegistry.getEntityFetcher(source); EntitiesRequestContext context = @@ -325,7 +326,7 @@ public EntityResponse visit(SelectionNode selectionNode) { .getExpressionContext() .getSourceToTimeAggregationMap() .get(source)) - .setFilter(filter) + .setFilter(addSourceFilters(executionContext, source, filter)) .build(); IEntityFetcher entityFetcher = queryHandlerRegistry.getEntityFetcher(source); EntitiesRequestContext requestContext = @@ -357,16 +358,18 @@ public EntityResponse visit(SelectionNode selectionNode) { private Filter addSourceFilters( EntityExecutionContext executionContext, String source, Filter filter) { - Filter andChildFilter = - executionContext.getExpressionContext().getSourceToAndFilterMap().get(source); - if (andChildFilter == null) { - return filter; - } - return Filter.newBuilder() - .setOperator(Operator.AND) - .addChildFilter(filter) - .addAllChildFilter(andChildFilter.getChildFilterList()) - .build(); + Optional sourceFilter = + Optional.ofNullable( + executionContext.getExpressionContext().getSourceToFilterMap().get(source)); + return sourceFilter + .map( + value -> + Filter.newBuilder() + .setOperator(Operator.AND) + .addChildFilter(filter) + .addChildFilter(value) + .build()) + .orElse(filter); } Filter constructFilterFromChildNodesResult(EntityFetcherResponse result) { diff --git a/gateway-service-impl/src/main/java/org/hypertrace/gateway/service/entity/query/visitor/SourceFilterVisitor.java b/gateway-service-impl/src/main/java/org/hypertrace/gateway/service/entity/query/visitor/SourceFilterVisitor.java new file mode 100644 index 00000000..2919451d --- /dev/null +++ b/gateway-service-impl/src/main/java/org/hypertrace/gateway/service/entity/query/visitor/SourceFilterVisitor.java @@ -0,0 +1,76 @@ +package org.hypertrace.gateway.service.entity.query.visitor; + +import org.hypertrace.gateway.service.common.util.TimeRangeFilterUtil; +import org.hypertrace.gateway.service.entity.query.AndNode; +import org.hypertrace.gateway.service.entity.query.DataFetcherNode; +import org.hypertrace.gateway.service.entity.query.EntityExecutionContext; +import org.hypertrace.gateway.service.entity.query.NoOpNode; +import org.hypertrace.gateway.service.entity.query.OrNode; +import org.hypertrace.gateway.service.entity.query.PaginateOnlyNode; +import org.hypertrace.gateway.service.entity.query.QueryNode; +import org.hypertrace.gateway.service.entity.query.SelectionNode; +import org.hypertrace.gateway.service.entity.query.SortAndPaginateNode; +import org.hypertrace.gateway.service.v1.common.Filter; + +/** Visitor that stores the filter tree per source in the query */ +public class SourceFilterVisitor implements Visitor { + + private final EntityExecutionContext executionContext; + + public SourceFilterVisitor(EntityExecutionContext executionContext) { + this.executionContext = executionContext; + } + + @Override + public QueryNode visit(DataFetcherNode dataFetcherNode) { + Filter timeRangeFilter = + TimeRangeFilterUtil.addTimeRangeFilter( + executionContext.getTimestampAttributeId(), + Filter.newBuilder().build(), + executionContext.getEntitiesRequest().getStartTimeMillis(), + executionContext.getEntitiesRequest().getEndTimeMillis()); + // we don't need to store timestamp filter + if (!timeRangeFilter.equals(dataFetcherNode.getFilter())) { + // if it is not a time-range filter, then we don't need to store it + executionContext + .getExpressionContext() + .putSourceToFilterMap(dataFetcherNode.getSource(), dataFetcherNode.getFilter()); + } + return dataFetcherNode; + } + + @Override + public QueryNode visit(AndNode andNode) { + andNode.getChildNodes().forEach(childNode -> childNode.acceptVisitor(this)); + return andNode; + } + + @Override + public QueryNode visit(OrNode orNode) { + orNode.getChildNodes().forEach(childNode -> childNode.acceptVisitor(this)); + return orNode; + } + + @Override + public QueryNode visit(SelectionNode selectionNode) { + selectionNode.getChildNode().acceptVisitor(this); + return selectionNode; + } + + @Override + public QueryNode visit(SortAndPaginateNode sortAndPaginateNode) { + sortAndPaginateNode.getChildNode().acceptVisitor(this); + return sortAndPaginateNode; + } + + @Override + public QueryNode visit(NoOpNode noOpNode) { + return noOpNode; + } + + @Override + public QueryNode visit(PaginateOnlyNode paginateOnlyNode) { + paginateOnlyNode.getChildNode().acceptVisitor(this); + return paginateOnlyNode; + } +} diff --git a/gateway-service-impl/src/test/resources/query-service-requests-and-responses/entity/backend-distinct-count.json b/gateway-service-impl/src/test/resources/query-service-requests-and-responses/entity/backend-distinct-count.json index 0182f968..f40cd04f 100644 --- a/gateway-service-impl/src/test/resources/query-service-requests-and-responses/entity/backend-distinct-count.json +++ b/gateway-service-impl/src/test/resources/query-service-requests-and-responses/entity/backend-distinct-count.json @@ -244,35 +244,37 @@ } } }, { - "lhs": { - "attributeExpression": { - "attributeId": "BACKEND.environment", - "alias": "environment" - } - }, - "operator": "EQ", - "rhs": { - "literal": { - "value": { - "string": "testenv" + "childFilter": [{ + "lhs": { + "attributeExpression": { + "attributeId": "BACKEND.environment", + "alias": "environment" + } + }, + "operator": "EQ", + "rhs": { + "literal": { + "value": { + "string": "testenv" + } } } - } - }, { - "lhs": { - "attributeExpression": { - "attributeId": "BACKEND.backendApiId", - "alias": "backendApiId" - } - }, - "operator": "NEQ", - "rhs": { - "literal": { - "value": { - "string": "null" + }, { + "lhs": { + "attributeExpression": { + "attributeId": "BACKEND.backendApiId", + "alias": "backendApiId" + } + }, + "operator": "NEQ", + "rhs": { + "literal": { + "value": { + "string": "null" + } } } - } + }] }] }] }, From 9f12153a243616263b8e84665e19d4d4b20703b6 Mon Sep 17 00:00:00 2001 From: saxenakshitiz Date: Thu, 23 May 2024 21:49:42 +0530 Subject: [PATCH 4/6] minor cleanup --- .../service/common/ExpressionContext.java | 41 ------------------- .../query/visitor/SourceFilterVisitor.java | 2 +- 2 files changed, 1 insertion(+), 42 deletions(-) diff --git a/gateway-service-impl/src/main/java/org/hypertrace/gateway/service/common/ExpressionContext.java b/gateway-service-impl/src/main/java/org/hypertrace/gateway/service/common/ExpressionContext.java index d7b73fff..59005437 100644 --- a/gateway-service-impl/src/main/java/org/hypertrace/gateway/service/common/ExpressionContext.java +++ b/gateway-service-impl/src/main/java/org/hypertrace/gateway/service/common/ExpressionContext.java @@ -1,8 +1,6 @@ package org.hypertrace.gateway.service.common; -import static java.util.Collections.emptyMap; import static java.util.function.Predicate.not; -import static org.hypertrace.core.attribute.service.v1.AttributeSource.QS; import com.google.common.collect.ImmutableMap; import com.google.common.collect.Sets; @@ -633,45 +631,6 @@ private static Set getIntersectingSourceSets( .orElse(Collections.emptySet()); } - private Map buildSourceToAndFilterMap(Filter filter) { - Operator operator = filter.getOperator(); - if (operator == Operator.AND) { - return filter.getChildFilterList().stream() - .map(this::buildSourceToAndFilterMap) - .flatMap(map -> map.entrySet().stream()) - .collect( - Collectors.toUnmodifiableMap( - Map.Entry::getKey, - Map.Entry::getValue, - (value1, value2) -> - Filter.newBuilder() - .setOperator(Operator.AND) - .addChildFilter(value1) - .addChildFilter(value2) - .build())); - - } else if (operator == Operator.OR) { - return Collections.emptyMap(); - } else { - List attributeSources = getAttributeSources(filter.getLhs()); - if (attributeSources.isEmpty()) { - return emptyMap(); - } - - return attributeSources.contains(QS) - ? Map.of(QS.name(), filter) - : Map.of(attributeSources.get(0).name(), filter); - } - } - - private List getAttributeSources(Expression expression) { - Set attributeIds = ExpressionReader.extractAttributeIds(expression); - return attributeIds.stream() - .map(attributeId -> attributeMetadataMap.get(attributeId).getSourcesList()) - .flatMap(Collection::stream) - .collect(Collectors.toUnmodifiableList()); - } - @Override public String toString() { return "ExpressionContext{" diff --git a/gateway-service-impl/src/main/java/org/hypertrace/gateway/service/entity/query/visitor/SourceFilterVisitor.java b/gateway-service-impl/src/main/java/org/hypertrace/gateway/service/entity/query/visitor/SourceFilterVisitor.java index 2919451d..daa99d28 100644 --- a/gateway-service-impl/src/main/java/org/hypertrace/gateway/service/entity/query/visitor/SourceFilterVisitor.java +++ b/gateway-service-impl/src/main/java/org/hypertrace/gateway/service/entity/query/visitor/SourceFilterVisitor.java @@ -26,7 +26,7 @@ public QueryNode visit(DataFetcherNode dataFetcherNode) { Filter timeRangeFilter = TimeRangeFilterUtil.addTimeRangeFilter( executionContext.getTimestampAttributeId(), - Filter.newBuilder().build(), + Filter.getDefaultInstance(), executionContext.getEntitiesRequest().getStartTimeMillis(), executionContext.getEntitiesRequest().getEndTimeMillis()); // we don't need to store timestamp filter From 8e82c0420c66839458e34a8db9bcca9ab4f99bf7 Mon Sep 17 00:00:00 2001 From: saxenakshitiz Date: Fri, 14 Jun 2024 17:10:52 +0530 Subject: [PATCH 5/6] address review comments --- .../service/common/ExpressionContext.java | 54 +++++++++++-- .../entity/query/ExecutionTreeBuilder.java | 80 +++---------------- .../query/visitor/ExecutionVisitor.java | 14 ++-- .../query/visitor/SourceFilterVisitor.java | 76 ------------------ .../query/ExecutionTreeBuilderTest.java | 20 ++--- 5 files changed, 78 insertions(+), 166 deletions(-) delete mode 100644 gateway-service-impl/src/main/java/org/hypertrace/gateway/service/entity/query/visitor/SourceFilterVisitor.java diff --git a/gateway-service-impl/src/main/java/org/hypertrace/gateway/service/common/ExpressionContext.java b/gateway-service-impl/src/main/java/org/hypertrace/gateway/service/common/ExpressionContext.java index 59005437..4a00fd67 100644 --- a/gateway-service-impl/src/main/java/org/hypertrace/gateway/service/common/ExpressionContext.java +++ b/gateway-service-impl/src/main/java/org/hypertrace/gateway/service/common/ExpressionContext.java @@ -1,6 +1,8 @@ package org.hypertrace.gateway.service.common; +import static java.util.Collections.emptyMap; import static java.util.function.Predicate.not; +import static org.hypertrace.core.attribute.service.v1.AttributeSource.QS; import com.google.common.collect.ImmutableMap; import com.google.common.collect.Sets; @@ -58,7 +60,7 @@ public class ExpressionContext { private ImmutableMap> sourceToFilterExpressionMap; private ImmutableMap> sourceToFilterAttributeMap; private ImmutableMap> filterAttributeToSourceMap; - private Map sourceToFilterMap; + private Map sourceToFilterMap; // and filter private boolean isAndFilter; @@ -92,9 +94,10 @@ public ExpressionContext( buildSourceToOrderByExpressionMaps(); buildSourceToGroupByExpressionMaps(); - this.sourceToFilterMap = new HashMap<>(); - this.isAndFilter = gatewayServiceConfig.isEntityAndFilterEnabled() && isAndFilter(filter); + // build source to filter map only if we only have AND filter + this.sourceToFilterMap = + isAndFilter(filter) ? buildSourceToAndFilterMap(filter) : Collections.emptyMap(); } public Map> getSourceToSelectionExpressionMap() { @@ -109,14 +112,10 @@ public void setSourceToSelectionExpressionMap( .build(); } - public Map getSourceToFilterMap() { + public Map getSourceToFilterMap() { return sourceToFilterMap; } - public void putSourceToFilterMap(String source, Filter filter) { - sourceToFilterMap.put(source, filter); - } - public Map> getSourceToSelectionAttributeMap() { return sourceToSelectionAttributeMap; } @@ -631,6 +630,45 @@ private static Set getIntersectingSourceSets( .orElse(Collections.emptySet()); } + private Map buildSourceToAndFilterMap(Filter filter) { + Operator operator = filter.getOperator(); + if (operator == Operator.AND) { + return filter.getChildFilterList().stream() + .map(this::buildSourceToAndFilterMap) + .flatMap(map -> map.entrySet().stream()) + .collect( + Collectors.toUnmodifiableMap( + Map.Entry::getKey, + Map.Entry::getValue, + (value1, value2) -> + Filter.newBuilder() + .setOperator(Operator.AND) + .addChildFilter(value1) + .addChildFilter(value2) + .build())); + + } else if (operator == Operator.OR) { + return Collections.emptyMap(); + } else { + List attributeSources = getAttributeSources(filter.getLhs()); + if (attributeSources.isEmpty()) { + return emptyMap(); + } + + return attributeSources.contains(QS) + ? Map.of(QS, filter) + : Map.of(attributeSources.get(0), filter); + } + } + + public List getAttributeSources(Expression expression) { + Set attributeIds = ExpressionReader.extractAttributeIds(expression); + return attributeIds.stream() + .map(attributeId -> attributeMetadataMap.get(attributeId).getSourcesList()) + .flatMap(Collection::stream) + .collect(Collectors.toUnmodifiableList()); + } + @Override public String toString() { return "ExpressionContext{" diff --git a/gateway-service-impl/src/main/java/org/hypertrace/gateway/service/entity/query/ExecutionTreeBuilder.java b/gateway-service-impl/src/main/java/org/hypertrace/gateway/service/entity/query/ExecutionTreeBuilder.java index d8bda4b4..3fd5fba6 100644 --- a/gateway-service-impl/src/main/java/org/hypertrace/gateway/service/entity/query/ExecutionTreeBuilder.java +++ b/gateway-service-impl/src/main/java/org/hypertrace/gateway/service/entity/query/ExecutionTreeBuilder.java @@ -1,7 +1,6 @@ package org.hypertrace.gateway.service.entity.query; import static java.util.Collections.emptyList; -import static java.util.Collections.emptyMap; import static java.util.Collections.unmodifiableList; import static org.hypertrace.core.attribute.service.v1.AttributeSource.EDS; import static org.hypertrace.core.attribute.service.v1.AttributeSource.QS; @@ -9,25 +8,18 @@ import com.google.common.annotations.VisibleForTesting; import java.util.ArrayList; import java.util.Collection; -import java.util.Collections; -import java.util.HashMap; import java.util.List; import java.util.Map; -import java.util.Map.Entry; import java.util.Optional; import java.util.Set; import java.util.stream.Collectors; import java.util.stream.Stream; -import org.hypertrace.core.attribute.service.v1.AttributeMetadata; import org.hypertrace.core.attribute.service.v1.AttributeSource; import org.hypertrace.gateway.service.common.ExpressionContext; -import org.hypertrace.gateway.service.common.util.ExpressionReader; import org.hypertrace.gateway.service.common.util.TimeRangeFilterUtil; import org.hypertrace.gateway.service.entity.query.visitor.ExecutionContextBuilderVisitor; import org.hypertrace.gateway.service.entity.query.visitor.FilterOptimizingVisitor; import org.hypertrace.gateway.service.entity.query.visitor.PrintVisitor; -import org.hypertrace.gateway.service.entity.query.visitor.SourceFilterVisitor; -import org.hypertrace.gateway.service.v1.common.Expression; import org.hypertrace.gateway.service.v1.common.Filter; import org.hypertrace.gateway.service.v1.common.Operator; import org.hypertrace.gateway.service.v1.common.OrderByExpression; @@ -40,18 +32,11 @@ public class ExecutionTreeBuilder { private static final Logger LOG = LoggerFactory.getLogger(ExecutionTreeBuilder.class); - private final Map attributeMetadataMap; private final EntityExecutionContext executionContext; private final Set sourceSetsIfFilterAndOrderByAreFromSameSourceSets; public ExecutionTreeBuilder(EntityExecutionContext executionContext) { this.executionContext = executionContext; - this.attributeMetadataMap = - executionContext - .getAttributeMetadataProvider() - .getAttributesMetadata( - executionContext.getEntitiesRequestContext(), - executionContext.getEntitiesRequest().getEntityType()); this.sourceSetsIfFilterAndOrderByAreFromSameSourceSets = ExpressionContext.getSourceSetsIfFilterAndOrderByAreFromSameSourceSets( @@ -133,7 +118,7 @@ public QueryNode build() { ExecutionTreeUtils.removeDuplicateSelectionAttributes(executionContext, QS.name()); - QueryNode filterTree = buildFilterTree(executionContext, entitiesRequest.getFilter()); + QueryNode filterTree = buildFilterTreeNode(executionContext, entitiesRequest.getFilter()); if (LOG.isDebugEnabled()) { LOG.debug("Filter Tree:{}", filterTree.acceptVisitor(new PrintVisitor())); } @@ -217,7 +202,7 @@ private QueryNode buildExecutionTreeForEdsFilterAndSelection() { QueryNode buildExecutionTree(EntityExecutionContext executionContext, QueryNode filterTree) { QueryNode rootNode = filterTree; // set up source filter to be used during selections - filterTree.acceptVisitor(new SourceFilterVisitor(executionContext)); + // filterTree.acceptVisitor(new SourceFilterVisitor(executionContext)); // Select attributes from sources in order by but not part of the filter tree Set attrSourcesForOrderBy = executionContext.getPendingSelectionSourcesForOrderBy(); if (!attrSourcesForOrderBy.isEmpty()) { @@ -271,8 +256,7 @@ QueryNode buildExecutionTree(EntityExecutionContext executionContext, QueryNode return rootNode; } - @VisibleForTesting - QueryNode buildFilterTree(EntityExecutionContext context, Filter filter) { + QueryNode buildFilterTreeNode(EntityExecutionContext context, Filter filter) { EntitiesRequest entitiesRequest = executionContext.getEntitiesRequest(); // Convert the time range into a filter and set it on the request so that all downstream // components needn't treat it specially @@ -284,13 +268,12 @@ QueryNode buildFilterTree(EntityExecutionContext context, Filter filter) { entitiesRequest.getEndTimeMillis()); boolean isAndFilter = executionContext.getExpressionContext().isAndFilter(); - return isAndFilter - ? buildAndFilterTree(entitiesRequest) - : buildFilterTree(entitiesRequest, timeRangeFilter); + return isAndFilter ? buildAndFilterTree(context) : buildFilterTree(context, timeRangeFilter); } @VisibleForTesting - QueryNode buildFilterTree(EntitiesRequest entitiesRequest, Filter filter) { + QueryNode buildFilterTree(EntityExecutionContext context, Filter filter) { + EntitiesRequest entitiesRequest = context.getEntitiesRequest(); if (filter.equals(Filter.getDefaultInstance())) { return new NoOpNode(); } @@ -298,15 +281,16 @@ QueryNode buildFilterTree(EntitiesRequest entitiesRequest, Filter filter) { if (operator == Operator.AND) { return new AndNode( filter.getChildFilterList().stream() - .map(childFilter -> buildFilterTree(entitiesRequest, childFilter)) + .map(childFilter -> buildFilterTree(context, childFilter)) .collect(Collectors.toList())); } else if (operator == Operator.OR) { return new OrNode( filter.getChildFilterList().stream() - .map(childFilter -> buildFilterTree(entitiesRequest, childFilter)) + .map(childFilter -> buildFilterTree(context, childFilter)) .collect(Collectors.toList())); } else { - List sources = getAttributeSources(filter.getLhs()); + List sources = + context.getExpressionContext().getAttributeSources(filter.getLhs()); // if the filter by and order by are from QS, pagination can be pushed down to QS // There will always be a DataFetcherNode for QS, because the results are always fetched @@ -322,7 +306,8 @@ QueryNode buildFilterTree(EntitiesRequest entitiesRequest, Filter filter) { } // filters and order by on QS, but you can still have selection on EDS - QueryNode buildAndFilterTree(EntitiesRequest entitiesRequest) { + QueryNode buildAndFilterTree(EntityExecutionContext context) { + EntitiesRequest entitiesRequest = context.getEntitiesRequest(); // If the filter by and order by are from QS (and selections are on other sources), pagination // can be pushed down to QS // Since the filter and order by are from QS, there won't be any filter on other @@ -333,7 +318,7 @@ QueryNode buildAndFilterTree(EntitiesRequest entitiesRequest) { } Map sourceToAndFilterMap = - new HashMap<>(buildSourceToAndFilterMap(entitiesRequest.getFilter())); + Map.copyOf(context.getExpressionContext().getSourceToFilterMap()); // qs node as the pivot node to fetch time range data QueryNode qsNode = @@ -390,37 +375,6 @@ QueryNode buildAndFilterTree(EntitiesRequest entitiesRequest) { } } - private Map buildSourceToAndFilterMap(Filter filter) { - Operator operator = filter.getOperator(); - if (operator == Operator.AND) { - return filter.getChildFilterList().stream() - .map(this::buildSourceToAndFilterMap) - .flatMap(map -> map.entrySet().stream()) - .collect( - Collectors.toUnmodifiableMap( - Entry::getKey, - Entry::getValue, - (value1, value2) -> - Filter.newBuilder() - .setOperator(Operator.AND) - .addChildFilter(value1) - .addChildFilter(value2) - .build())); - - } else if (operator == Operator.OR) { - return Collections.emptyMap(); - } else { - List attributeSources = getAttributeSources(filter.getLhs()); - if (attributeSources.isEmpty()) { - return emptyMap(); - } - - return attributeSources.contains(QS) - ? Map.of(QS, filter) - : Map.of(attributeSources.get(0), filter); - } - } - private QueryNode checkAndAddSortAndPaginationNode( QueryNode childNode, EntityExecutionContext executionContext) { EntitiesRequest entitiesRequest = executionContext.getEntitiesRequest(); @@ -482,12 +436,4 @@ private QueryNode createQsDataFetcherNodeWithLimitAndOffset(EntitiesRequest enti private QueryNode createPaginateOnlyNode(QueryNode queryNode, EntitiesRequest entitiesRequest) { return new PaginateOnlyNode(queryNode, entitiesRequest.getLimit(), entitiesRequest.getOffset()); } - - public List getAttributeSources(Expression expression) { - Set attributeIds = ExpressionReader.extractAttributeIds(expression); - return attributeIds.stream() - .map(attributeId -> attributeMetadataMap.get(attributeId).getSourcesList()) - .flatMap(Collection::stream) - .collect(Collectors.toUnmodifiableList()); - } } diff --git a/gateway-service-impl/src/main/java/org/hypertrace/gateway/service/entity/query/visitor/ExecutionVisitor.java b/gateway-service-impl/src/main/java/org/hypertrace/gateway/service/entity/query/visitor/ExecutionVisitor.java index e7326d4d..d9baca90 100644 --- a/gateway-service-impl/src/main/java/org/hypertrace/gateway/service/entity/query/visitor/ExecutionVisitor.java +++ b/gateway-service-impl/src/main/java/org/hypertrace/gateway/service/entity/query/visitor/ExecutionVisitor.java @@ -16,6 +16,7 @@ import java.util.concurrent.ExecutorService; import java.util.stream.Collectors; import java.util.stream.IntStream; +import org.hypertrace.core.attribute.service.v1.AttributeSource; import org.hypertrace.gateway.service.common.datafetcher.EntityFetcherResponse; import org.hypertrace.gateway.service.common.datafetcher.EntityResponse; import org.hypertrace.gateway.service.common.datafetcher.IEntityFetcher; @@ -358,16 +359,19 @@ public EntityResponse visit(SelectionNode selectionNode) { private Filter addSourceFilters( EntityExecutionContext executionContext, String source, Filter filter) { - Optional sourceFilter = + Optional sourceFilterOptional = Optional.ofNullable( - executionContext.getExpressionContext().getSourceToFilterMap().get(source)); - return sourceFilter + executionContext + .getExpressionContext() + .getSourceToFilterMap() + .get(AttributeSource.valueOf(source))); + return sourceFilterOptional .map( - value -> + sourceFilter -> Filter.newBuilder() .setOperator(Operator.AND) .addChildFilter(filter) - .addChildFilter(value) + .addChildFilter(sourceFilter) .build()) .orElse(filter); } diff --git a/gateway-service-impl/src/main/java/org/hypertrace/gateway/service/entity/query/visitor/SourceFilterVisitor.java b/gateway-service-impl/src/main/java/org/hypertrace/gateway/service/entity/query/visitor/SourceFilterVisitor.java deleted file mode 100644 index daa99d28..00000000 --- a/gateway-service-impl/src/main/java/org/hypertrace/gateway/service/entity/query/visitor/SourceFilterVisitor.java +++ /dev/null @@ -1,76 +0,0 @@ -package org.hypertrace.gateway.service.entity.query.visitor; - -import org.hypertrace.gateway.service.common.util.TimeRangeFilterUtil; -import org.hypertrace.gateway.service.entity.query.AndNode; -import org.hypertrace.gateway.service.entity.query.DataFetcherNode; -import org.hypertrace.gateway.service.entity.query.EntityExecutionContext; -import org.hypertrace.gateway.service.entity.query.NoOpNode; -import org.hypertrace.gateway.service.entity.query.OrNode; -import org.hypertrace.gateway.service.entity.query.PaginateOnlyNode; -import org.hypertrace.gateway.service.entity.query.QueryNode; -import org.hypertrace.gateway.service.entity.query.SelectionNode; -import org.hypertrace.gateway.service.entity.query.SortAndPaginateNode; -import org.hypertrace.gateway.service.v1.common.Filter; - -/** Visitor that stores the filter tree per source in the query */ -public class SourceFilterVisitor implements Visitor { - - private final EntityExecutionContext executionContext; - - public SourceFilterVisitor(EntityExecutionContext executionContext) { - this.executionContext = executionContext; - } - - @Override - public QueryNode visit(DataFetcherNode dataFetcherNode) { - Filter timeRangeFilter = - TimeRangeFilterUtil.addTimeRangeFilter( - executionContext.getTimestampAttributeId(), - Filter.getDefaultInstance(), - executionContext.getEntitiesRequest().getStartTimeMillis(), - executionContext.getEntitiesRequest().getEndTimeMillis()); - // we don't need to store timestamp filter - if (!timeRangeFilter.equals(dataFetcherNode.getFilter())) { - // if it is not a time-range filter, then we don't need to store it - executionContext - .getExpressionContext() - .putSourceToFilterMap(dataFetcherNode.getSource(), dataFetcherNode.getFilter()); - } - return dataFetcherNode; - } - - @Override - public QueryNode visit(AndNode andNode) { - andNode.getChildNodes().forEach(childNode -> childNode.acceptVisitor(this)); - return andNode; - } - - @Override - public QueryNode visit(OrNode orNode) { - orNode.getChildNodes().forEach(childNode -> childNode.acceptVisitor(this)); - return orNode; - } - - @Override - public QueryNode visit(SelectionNode selectionNode) { - selectionNode.getChildNode().acceptVisitor(this); - return selectionNode; - } - - @Override - public QueryNode visit(SortAndPaginateNode sortAndPaginateNode) { - sortAndPaginateNode.getChildNode().acceptVisitor(this); - return sortAndPaginateNode; - } - - @Override - public QueryNode visit(NoOpNode noOpNode) { - return noOpNode; - } - - @Override - public QueryNode visit(PaginateOnlyNode paginateOnlyNode) { - paginateOnlyNode.getChildNode().acceptVisitor(this); - return paginateOnlyNode; - } -} diff --git a/gateway-service-impl/src/test/java/org/hypertrace/gateway/service/entity/query/ExecutionTreeBuilderTest.java b/gateway-service-impl/src/test/java/org/hypertrace/gateway/service/entity/query/ExecutionTreeBuilderTest.java index a6139e6b..0ef5f953 100644 --- a/gateway-service-impl/src/test/java/org/hypertrace/gateway/service/entity/query/ExecutionTreeBuilderTest.java +++ b/gateway-service-impl/src/test/java/org/hypertrace/gateway/service/entity/query/ExecutionTreeBuilderTest.java @@ -194,7 +194,7 @@ public void testOptimizedFilterTreeBuilderSimpleFilter() { EntitiesRequest entitiesRequest = buildEntitiesRequest(filter); EntityExecutionContext executionContext = getExecutionContext(entitiesRequest); ExecutionTreeBuilder executionTreeBuilder = new ExecutionTreeBuilder(executionContext); - QueryNode queryNode = executionTreeBuilder.buildFilterTree(entitiesRequest, filter); + QueryNode queryNode = executionTreeBuilder.buildFilterTree(executionContext, filter); QueryNode optimizedQueryNode = queryNode.acceptVisitor(new FilterOptimizingVisitor()); assertNotNull(optimizedQueryNode); assertTrue(optimizedQueryNode instanceof DataFetcherNode); @@ -213,7 +213,7 @@ public void testOptimizedFilterTreeBuilderAndOrFilterSingleDataSource() { EntitiesRequest entitiesRequest = buildEntitiesRequest(filter); EntityExecutionContext executionContext = getExecutionContext(entitiesRequest); ExecutionTreeBuilder executionTreeBuilder = new ExecutionTreeBuilder(executionContext); - QueryNode queryNode = executionTreeBuilder.buildFilterTree(entitiesRequest, filter); + QueryNode queryNode = executionTreeBuilder.buildFilterTree(executionContext, filter); assertNotNull(queryNode); assertTrue(queryNode instanceof AndNode); QueryNode optimizedNode = queryNode.acceptVisitor(new FilterOptimizingVisitor()); @@ -232,7 +232,7 @@ public void testOptimizedFilterTreeBuilderAndOrFilterSingleDataSource() { EntitiesRequest entitiesRequest = buildEntitiesRequest(filter); EntityExecutionContext executionContext = getExecutionContext(entitiesRequest); ExecutionTreeBuilder executionTreeBuilder = new ExecutionTreeBuilder(executionContext); - QueryNode queryNode = executionTreeBuilder.buildFilterTree(entitiesRequest, filter); + QueryNode queryNode = executionTreeBuilder.buildFilterTree(executionContext, filter); assertNotNull(queryNode); assertTrue(queryNode instanceof OrNode); QueryNode optimizedNode = queryNode.acceptVisitor(new FilterOptimizingVisitor()); @@ -285,7 +285,7 @@ public void testOptimizedFilterTreeBuilderAndOrFilterMultiDataSource() { EntitiesRequest entitiesRequest = buildEntitiesRequest(filter); EntityExecutionContext executionContext = getExecutionContext(entitiesRequest); ExecutionTreeBuilder executionTreeBuilder = new ExecutionTreeBuilder(executionContext); - QueryNode queryNode = executionTreeBuilder.buildFilterTree(entitiesRequest, filter); + QueryNode queryNode = executionTreeBuilder.buildFilterTree(executionContext, filter); assertNotNull(queryNode); assertTrue(queryNode instanceof AndNode); QueryNode optimizedNode = queryNode.acceptVisitor(new FilterOptimizingVisitor()); @@ -311,7 +311,7 @@ public void testOptimizedFilterTreeBuilderAndOrFilterMultiDataSource() { EntitiesRequest entitiesRequest = buildEntitiesRequest(filter); EntityExecutionContext executionContext = getExecutionContext(entitiesRequest); ExecutionTreeBuilder executionTreeBuilder = new ExecutionTreeBuilder(executionContext); - QueryNode queryNode = executionTreeBuilder.buildFilterTree(entitiesRequest, filter); + QueryNode queryNode = executionTreeBuilder.buildFilterTree(executionContext, filter); assertNotNull(queryNode); assertTrue(queryNode instanceof OrNode); QueryNode optimizedNode = queryNode.acceptVisitor(new FilterOptimizingVisitor()); @@ -360,7 +360,7 @@ public void testOptimizedFilterTreeBuilderNestedAndFilter() { EntitiesRequest entitiesRequest = buildEntitiesRequest(filter); EntityExecutionContext executionContext = getExecutionContext(entitiesRequest); ExecutionTreeBuilder executionTreeBuilder = new ExecutionTreeBuilder(executionContext); - QueryNode queryNode = executionTreeBuilder.buildFilterTree(entitiesRequest, filter); + QueryNode queryNode = executionTreeBuilder.buildFilterTree(executionContext, filter); assertNotNull(queryNode); QueryNode optimizedNode = queryNode.acceptVisitor(new FilterOptimizingVisitor()); assertNotNull(optimizedNode); @@ -385,7 +385,7 @@ public void testOptimizedFilterTreeBuilderNestedAndFilter() { EntitiesRequest entitiesRequest = buildEntitiesRequest(filter); EntityExecutionContext executionContext = getExecutionContext(entitiesRequest); ExecutionTreeBuilder executionTreeBuilder = new ExecutionTreeBuilder(executionContext); - QueryNode queryNode = executionTreeBuilder.buildFilterTree(entitiesRequest, filter); + QueryNode queryNode = executionTreeBuilder.buildFilterTree(executionContext, filter); assertNotNull(queryNode); QueryNode optimizedNode = queryNode.acceptVisitor(new FilterOptimizingVisitor()); assertNotNull(optimizedNode); @@ -412,7 +412,7 @@ public void testOptimizedFilterTreeBuilderNestedAndFilter() { EntitiesRequest entitiesRequest = buildEntitiesRequest(filter); EntityExecutionContext executionContext = getExecutionContext(entitiesRequest); ExecutionTreeBuilder executionTreeBuilder = new ExecutionTreeBuilder(executionContext); - QueryNode queryNode = executionTreeBuilder.buildFilterTree(entitiesRequest, filter); + QueryNode queryNode = executionTreeBuilder.buildFilterTree(executionContext, filter); assertNotNull(queryNode); QueryNode optimizedNode = queryNode.acceptVisitor(new FilterOptimizingVisitor()); assertNotNull(optimizedNode); @@ -442,7 +442,7 @@ public void testOptimizedFilterTreeBuilderNestedAndOrFilter() { EntitiesRequest entitiesRequest = buildEntitiesRequest(filter); EntityExecutionContext executionContext = getExecutionContext(entitiesRequest); ExecutionTreeBuilder executionTreeBuilder = new ExecutionTreeBuilder(executionContext); - QueryNode queryNode = executionTreeBuilder.buildFilterTree(entitiesRequest, filter); + QueryNode queryNode = executionTreeBuilder.buildFilterTree(executionContext, filter); assertNotNull(queryNode); QueryNode optimizedNode = queryNode.acceptVisitor(new FilterOptimizingVisitor()); assertNotNull(optimizedNode); @@ -467,7 +467,7 @@ public void testOptimizedFilterTreeBuilderNestedAndOrFilter() { EntitiesRequest entitiesRequest = buildEntitiesRequest(filter); EntityExecutionContext executionContext = getExecutionContext(entitiesRequest); ExecutionTreeBuilder executionTreeBuilder = new ExecutionTreeBuilder(executionContext); - QueryNode queryNode = executionTreeBuilder.buildFilterTree(entitiesRequest, filter); + QueryNode queryNode = executionTreeBuilder.buildFilterTree(executionContext, filter); assertNotNull(queryNode); QueryNode optimizedNode = queryNode.acceptVisitor(new FilterOptimizingVisitor()); assertNotNull(optimizedNode); From 9eae09e6092b9bf8a78aff0e0aa1ffac1c032037 Mon Sep 17 00:00:00 2001 From: saxenakshitiz Date: Fri, 14 Jun 2024 20:46:15 +0530 Subject: [PATCH 6/6] address review comments --- .../gateway/service/entity/query/ExecutionTreeBuilder.java | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/gateway-service-impl/src/main/java/org/hypertrace/gateway/service/entity/query/ExecutionTreeBuilder.java b/gateway-service-impl/src/main/java/org/hypertrace/gateway/service/entity/query/ExecutionTreeBuilder.java index 3fd5fba6..527c789b 100644 --- a/gateway-service-impl/src/main/java/org/hypertrace/gateway/service/entity/query/ExecutionTreeBuilder.java +++ b/gateway-service-impl/src/main/java/org/hypertrace/gateway/service/entity/query/ExecutionTreeBuilder.java @@ -8,6 +8,7 @@ import com.google.common.annotations.VisibleForTesting; import java.util.ArrayList; import java.util.Collection; +import java.util.HashMap; import java.util.List; import java.util.Map; import java.util.Optional; @@ -201,8 +202,6 @@ private QueryNode buildExecutionTreeForEdsFilterAndSelection() { @VisibleForTesting QueryNode buildExecutionTree(EntityExecutionContext executionContext, QueryNode filterTree) { QueryNode rootNode = filterTree; - // set up source filter to be used during selections - // filterTree.acceptVisitor(new SourceFilterVisitor(executionContext)); // Select attributes from sources in order by but not part of the filter tree Set attrSourcesForOrderBy = executionContext.getPendingSelectionSourcesForOrderBy(); if (!attrSourcesForOrderBy.isEmpty()) { @@ -318,7 +317,7 @@ QueryNode buildAndFilterTree(EntityExecutionContext context) { } Map sourceToAndFilterMap = - Map.copyOf(context.getExpressionContext().getSourceToFilterMap()); + new HashMap<>(context.getExpressionContext().getSourceToFilterMap()); // qs node as the pivot node to fetch time range data QueryNode qsNode =