From 74de0f2829637122cb238e52b8763d6e858b7ece Mon Sep 17 00:00:00 2001 From: Sarthak Singhal Date: Fri, 17 Dec 2021 14:00:28 +0530 Subject: [PATCH 01/23] added sup for attr expr in execution context --- .../core/query/service/ExecutionContext.java | 22 +++++++++++++------ .../core/query/service/QueryRequestUtil.java | 16 ++++++++++++++ 2 files changed, 31 insertions(+), 7 deletions(-) diff --git a/query-service-impl/src/main/java/org/hypertrace/core/query/service/ExecutionContext.java b/query-service-impl/src/main/java/org/hypertrace/core/query/service/ExecutionContext.java index 26559593..f8629d23 100644 --- a/query-service-impl/src/main/java/org/hypertrace/core/query/service/ExecutionContext.java +++ b/query-service-impl/src/main/java/org/hypertrace/core/query/service/ExecutionContext.java @@ -1,5 +1,8 @@ package org.hypertrace.core.query.service; +import static org.hypertrace.core.query.service.QueryRequestUtil.getAlias; +import static org.hypertrace.core.query.service.QueryRequestUtil.getLogicalColumnName; + import com.google.common.base.Supplier; import com.google.common.base.Suppliers; import java.time.Duration; @@ -15,7 +18,6 @@ import java.util.Optional; import java.util.Set; import java.util.concurrent.TimeUnit; -import org.hypertrace.core.query.service.api.ColumnIdentifier; import org.hypertrace.core.query.service.api.ColumnMetadata; import org.hypertrace.core.query.service.api.Expression; import org.hypertrace.core.query.service.api.Expression.ValueCase; @@ -137,12 +139,12 @@ private ColumnMetadata toColumnMetadata(Expression expression) { ValueCase valueCase = expression.getValueCase(); switch (valueCase) { case COLUMNIDENTIFIER: - ColumnIdentifier columnIdentifier = expression.getColumnIdentifier(); - String alias = columnIdentifier.getAlias(); + case ATTRIBUTE_EXPRESSION: + String alias = getAlias(expression); if (alias != null && alias.trim().length() > 0) { builder.setColumnName(alias); } else { - builder.setColumnName(columnIdentifier.getColumnName()); + builder.setColumnName(getLogicalColumnName(expression)); } builder.setValueType(ValueType.STRING); builder.setIsRepeated(false); @@ -172,8 +174,8 @@ private void extractColumns(List columns, Expression expression) { ValueCase valueCase = expression.getValueCase(); switch (valueCase) { case COLUMNIDENTIFIER: - ColumnIdentifier columnIdentifier = expression.getColumnIdentifier(); - columns.add(columnIdentifier.getColumnName()); + case ATTRIBUTE_EXPRESSION: + columns.add(getLogicalColumnName(expression)); break; case LITERAL: // no columns @@ -233,7 +235,13 @@ private Optional buildQueryTimeRange(Filter filter, String timeF } private boolean isMatchingFilter(Filter filter, String column, Collection operators) { - return column.equals(filter.getLhs().getColumnIdentifier().getColumnName()) + + // if not leaf filter, then return false + if (!filter.hasLhs()) { + return false; + } + + return column.equals(getLogicalColumnName(filter.getLhs())) && (operators.stream() .anyMatch(operator -> Objects.equals(operator, filter.getOperator()))); } diff --git a/query-service-impl/src/main/java/org/hypertrace/core/query/service/QueryRequestUtil.java b/query-service-impl/src/main/java/org/hypertrace/core/query/service/QueryRequestUtil.java index 0fb9cfa9..409f8292 100644 --- a/query-service-impl/src/main/java/org/hypertrace/core/query/service/QueryRequestUtil.java +++ b/query-service-impl/src/main/java/org/hypertrace/core/query/service/QueryRequestUtil.java @@ -139,4 +139,20 @@ public static String getLogicalColumnName(Expression expression) { + " expression type only"); } } + + public static String getAlias(Expression expression) { + switch (expression.getValueCase()) { + case COLUMNIDENTIFIER: + return expression.getColumnIdentifier().getAlias(); + case ATTRIBUTE_EXPRESSION: + return expression.getAttributeExpression().getAlias(); + default: + throw new IllegalArgumentException( + "Supports " + + ATTRIBUTE_EXPRESSION + + " and " + + COLUMNIDENTIFIER + + " expression type only"); + } + } } From 61f141c90a21b5f72d19f84908f31029355297b1 Mon Sep 17 00:00:00 2001 From: Sarthak Singhal Date: Mon, 20 Dec 2021 09:25:22 +0530 Subject: [PATCH 02/23] added sup for attr expr in projection transformation --- .../projection/ProjectionTransformation.java | 33 ++++++++++++++++--- 1 file changed, 29 insertions(+), 4 deletions(-) diff --git a/query-service-impl/src/main/java/org/hypertrace/core/query/service/projection/ProjectionTransformation.java b/query-service-impl/src/main/java/org/hypertrace/core/query/service/projection/ProjectionTransformation.java index 558afd96..3216bb61 100644 --- a/query-service-impl/src/main/java/org/hypertrace/core/query/service/projection/ProjectionTransformation.java +++ b/query-service-impl/src/main/java/org/hypertrace/core/query/service/projection/ProjectionTransformation.java @@ -30,6 +30,7 @@ import org.hypertrace.core.query.service.QueryFunctionConstants; import org.hypertrace.core.query.service.QueryRequestUtil; import org.hypertrace.core.query.service.QueryTransformation; +import org.hypertrace.core.query.service.api.AttributeExpression; import org.hypertrace.core.query.service.api.ColumnIdentifier; import org.hypertrace.core.query.service.api.Expression; import org.hypertrace.core.query.service.api.Filter; @@ -79,6 +80,8 @@ private Single transformExpression(Expression expression) { switch (expression.getValueCase()) { case COLUMNIDENTIFIER: return this.transformColumnIdentifier(expression.getColumnIdentifier()); + case ATTRIBUTE_EXPRESSION: + return this.transformAttributeExpression(expression.getAttributeExpression()); case FUNCTION: return this.transformFunction(expression.getFunction()) .map(expression.toBuilder()::setFunction) @@ -96,10 +99,19 @@ private Single transformExpression(Expression expression) { private Single transformColumnIdentifier(ColumnIdentifier columnIdentifier) { return this.projectAttributeIfPossible(columnIdentifier.getColumnName()) - .map(expression -> this.aliasToMatchOriginal(columnIdentifier, expression)) + .map(expression -> this.aliasToMatchOriginal(getOriginalKey(columnIdentifier), expression)) .defaultIfEmpty(Expression.newBuilder().setColumnIdentifier(columnIdentifier).build()); } + private Single transformAttributeExpression(AttributeExpression attributeExpression) { + return this.projectAttributeIfPossible(attributeExpression.getAttributeId()) + .map( + expression -> + this.aliasToMatchOriginal(getOriginalKey(attributeExpression), expression)) + .defaultIfEmpty( + Expression.newBuilder().setAttributeExpression(attributeExpression).build()); + } + private Single transformFunction(Function function) { return this.transformExpressionList(function.getArgumentsList()) .map(expressions -> function.toBuilder().clearArguments().addAllArguments(expressions)) @@ -267,15 +279,18 @@ private Single convertOperator(ProjectionOperator operator) { } } - private Expression aliasToMatchOriginal(ColumnIdentifier original, Expression newExpression) { - String originalKey = - original.getAlias().isEmpty() ? original.getColumnName() : original.getAlias(); + private Expression aliasToMatchOriginal(String originalKey, Expression newExpression) { switch (newExpression.getValueCase()) { case COLUMNIDENTIFIER: return newExpression.toBuilder() .setColumnIdentifier( newExpression.getColumnIdentifier().toBuilder().setAlias(originalKey)) .build(); + case ATTRIBUTE_EXPRESSION: + return newExpression.toBuilder() + .setAttributeExpression( + newExpression.getAttributeExpression().toBuilder().setAlias(originalKey)) + .build(); case FUNCTION: return newExpression.toBuilder() .setFunction(newExpression.getFunction().toBuilder().setAlias(originalKey)) @@ -417,4 +432,14 @@ private List createFilterForComplexAttributeExpressionFromOrderBy( .map(QueryRequestUtil::createContainsKeyFilter) .collect(Collectors.toList()); } + + private String getOriginalKey(AttributeExpression attributeExpression) { + String alias = attributeExpression.getAlias(); + return alias.isEmpty() ? attributeExpression.getAttributeId() : alias; + } + + private String getOriginalKey(ColumnIdentifier columnIdentifier) { + String alias = columnIdentifier.getAlias(); + return alias.isEmpty() ? columnIdentifier.getColumnName() : alias; + } } From 8bf4b47455637cdb6166ec3efa534d0aa960e067 Mon Sep 17 00:00:00 2001 From: Sarthak Singhal Date: Mon, 20 Dec 2021 09:51:32 +0530 Subject: [PATCH 03/23] added sup for attr expr in pinotbasedrequesthandler --- .../pinot/PinotBasedRequestHandler.java | 20 +++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) diff --git a/query-service-impl/src/main/java/org/hypertrace/core/query/service/pinot/PinotBasedRequestHandler.java b/query-service-impl/src/main/java/org/hypertrace/core/query/service/pinot/PinotBasedRequestHandler.java index 278ed173..2c40f9a3 100644 --- a/query-service-impl/src/main/java/org/hypertrace/core/query/service/pinot/PinotBasedRequestHandler.java +++ b/query-service-impl/src/main/java/org/hypertrace/core/query/service/pinot/PinotBasedRequestHandler.java @@ -1,6 +1,7 @@ package org.hypertrace.core.query.service.pinot; import static org.hypertrace.core.query.service.ConfigUtils.optionallyGet; +import static org.hypertrace.core.query.service.QueryRequestUtil.getLogicalColumnName; import com.google.common.base.Preconditions; import com.google.common.base.Stopwatch; @@ -225,10 +226,12 @@ && rhsHasLongValue(filter.getRhs())) { } private boolean lhsIsStartTimeAttribute(Expression lhs) { - return lhs.hasColumnIdentifier() - && startTimeAttributeName - .map(attributeName -> attributeName.equals(lhs.getColumnIdentifier().getColumnName())) - .orElse(false); + if (lhs.hasColumnIdentifier() || lhs.hasAttributeExpression()) { + return startTimeAttributeName + .map(attributeName -> attributeName.equals(getLogicalColumnName(lhs))) + .orElse(false); + } + return false; } private boolean rhsHasLongValue(Expression rhs) { @@ -245,7 +248,7 @@ private Set getMatchingViewFilterColumns( // return it. if (filter.getChildFilterCount() == 0) { return doesSingleViewFilterMatchLeafQueryFilter(viewFilterMap, filter) - ? Set.of(filter.getLhs().getColumnIdentifier().getColumnName()) + ? Set.of(getLogicalColumnName(filter.getLhs())) : Set.of(); } else { // 2. Internal filter node. Recursively get the matching nodes from children. @@ -274,14 +277,15 @@ private Set getMatchingViewFilterColumns( */ private boolean doesSingleViewFilterMatchLeafQueryFilter( Map viewFilterMap, Filter queryFilter) { - if (queryFilter.getLhs().getValueCase() != ValueCase.COLUMNIDENTIFIER) { + if (queryFilter.getLhs().getValueCase() != ValueCase.COLUMNIDENTIFIER + && queryFilter.getLhs().getValueCase() != ValueCase.ATTRIBUTE_EXPRESSION) { return false; } if (queryFilter.getOperator() != Operator.IN && queryFilter.getOperator() != Operator.EQ) { return false; } - String columnName = queryFilter.getLhs().getColumnIdentifier().getColumnName(); + String columnName = getLogicalColumnName(queryFilter.getLhs()); ViewColumnFilter viewColumnFilter = viewFilterMap.get(columnName); if (viewColumnFilter == null) { return false; @@ -469,7 +473,7 @@ private Filter removeViewColumnFilter( private Filter rewriteLeafFilter( Filter queryFilter, Map columnFilterMap) { ViewColumnFilter viewColumnFilter = - columnFilterMap.get(queryFilter.getLhs().getColumnIdentifier().getColumnName()); + columnFilterMap.get(getLogicalColumnName(queryFilter.getLhs())); // If the RHS of both the view filter and query filter match, return empty filter. if (viewColumnFilter != null && isEquals(viewColumnFilter.getValues(), queryFilter.getRhs())) { return Filter.getDefaultInstance(); From 0eedfb72d4855e662b9080d2bf75b61df787886d Mon Sep 17 00:00:00 2001 From: Sarthak Singhal Date: Mon, 20 Dec 2021 10:23:38 +0530 Subject: [PATCH 04/23] refactored --- .../core/query/service/ExecutionContext.java | 19 +------------------ .../core/query/service/QueryRequestUtil.java | 17 +++++++++++++++-- 2 files changed, 16 insertions(+), 20 deletions(-) diff --git a/query-service-impl/src/main/java/org/hypertrace/core/query/service/ExecutionContext.java b/query-service-impl/src/main/java/org/hypertrace/core/query/service/ExecutionContext.java index f8629d23..b6cd5b27 100644 --- a/query-service-impl/src/main/java/org/hypertrace/core/query/service/ExecutionContext.java +++ b/query-service-impl/src/main/java/org/hypertrace/core/query/service/ExecutionContext.java @@ -140,25 +140,8 @@ private ColumnMetadata toColumnMetadata(Expression expression) { switch (valueCase) { case COLUMNIDENTIFIER: case ATTRIBUTE_EXPRESSION: - String alias = getAlias(expression); - if (alias != null && alias.trim().length() > 0) { - builder.setColumnName(alias); - } else { - builder.setColumnName(getLogicalColumnName(expression)); - } - builder.setValueType(ValueType.STRING); - builder.setIsRepeated(false); - break; case FUNCTION: - Function function = expression.getFunction(); - alias = function.getAlias(); - if (alias != null && alias.trim().length() > 0) { - builder.setColumnName(alias); - } else { - // todo: handle recursive functions max(rollup(time,50) - // workaround is to use alias for now - builder.setColumnName(function.getFunctionName()); - } + builder.setColumnName(getAlias(expression)); builder.setValueType(ValueType.STRING); builder.setIsRepeated(false); break; diff --git a/query-service-impl/src/main/java/org/hypertrace/core/query/service/QueryRequestUtil.java b/query-service-impl/src/main/java/org/hypertrace/core/query/service/QueryRequestUtil.java index 409f8292..1518b170 100644 --- a/query-service-impl/src/main/java/org/hypertrace/core/query/service/QueryRequestUtil.java +++ b/query-service-impl/src/main/java/org/hypertrace/core/query/service/QueryRequestUtil.java @@ -2,6 +2,7 @@ import static org.hypertrace.core.query.service.api.Expression.ValueCase.ATTRIBUTE_EXPRESSION; import static org.hypertrace.core.query.service.api.Expression.ValueCase.COLUMNIDENTIFIER; +import static org.hypertrace.core.query.service.api.Expression.ValueCase.FUNCTION; import org.hypertrace.core.query.service.api.AttributeExpression; import org.hypertrace.core.query.service.api.ColumnIdentifier; @@ -143,13 +144,25 @@ public static String getLogicalColumnName(Expression expression) { public static String getAlias(Expression expression) { switch (expression.getValueCase()) { case COLUMNIDENTIFIER: - return expression.getColumnIdentifier().getAlias(); + return expression.getColumnIdentifier().getAlias().isBlank() + ? getLogicalColumnName(expression) + : expression.getColumnIdentifier().getAlias(); case ATTRIBUTE_EXPRESSION: - return expression.getAttributeExpression().getAlias(); + return expression.getAttributeExpression().getAlias().isBlank() + ? getLogicalColumnName(expression) + : expression.getAttributeExpression().getAlias(); + case FUNCTION: + // todo: handle recursive functions max(rollup(time,50) + // workaround is to use alias for now + return expression.getFunction().getAlias().isBlank() + ? expression.getFunction().getFunctionName() + : expression.getFunction().getAlias(); default: throw new IllegalArgumentException( "Supports " + ATTRIBUTE_EXPRESSION + + " , " + + FUNCTION + " and " + COLUMNIDENTIFIER + " expression type only"); From 78b71d0b02572d68a3abbbaf5fc5eeac1e54d7ca Mon Sep 17 00:00:00 2001 From: Sarthak Singhal Date: Mon, 20 Dec 2021 11:00:51 +0530 Subject: [PATCH 05/23] added unit test for execution context --- .../service/pinot/ExecutionContextTest.java | 73 +++++++++++++++++++ 1 file changed, 73 insertions(+) diff --git a/query-service-impl/src/test/java/org/hypertrace/core/query/service/pinot/ExecutionContextTest.java b/query-service-impl/src/test/java/org/hypertrace/core/query/service/pinot/ExecutionContextTest.java index 1a3513aa..b550977a 100644 --- a/query-service-impl/src/test/java/org/hypertrace/core/query/service/pinot/ExecutionContextTest.java +++ b/query-service-impl/src/test/java/org/hypertrace/core/query/service/pinot/ExecutionContextTest.java @@ -3,6 +3,7 @@ import static org.hypertrace.core.query.service.QueryRequestBuilderUtils.createFilter; import static org.hypertrace.core.query.service.QueryRequestBuilderUtils.createTimeColumnGroupByExpression; import static org.hypertrace.core.query.service.QueryRequestBuilderUtils.createTimeFilter; +import static org.hypertrace.core.query.service.QueryRequestUtil.createSimpleAttributeExpression; import static org.hypertrace.core.query.service.QueryRequestUtil.createStringLiteralValueExpression; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertNotNull; @@ -320,6 +321,78 @@ public void testSelectionsLinkedHashSet() { Expression.newBuilder().setFunction(minFunction).build(), selectionsIterator.next()); } + @Test + public void testSelectionsLinkedHashSetWithAttributeExpression() { + Builder builder = QueryRequest.newBuilder(); + // agg function with alias + Function count = + Function.newBuilder() + .setFunctionName("Count") + .setAlias("myCountAlias") + .addArguments(createSimpleAttributeExpression("Trace.id")) + .build(); + builder.addAggregation(Expression.newBuilder().setFunction(count)); + + // agg function without alias + Function minFunction = + Function.newBuilder() + .setFunctionName("MIN") + .addArguments(createSimpleAttributeExpression("Trace.duration")) + .build(); + builder.addAggregation(Expression.newBuilder().setFunction(minFunction)); + + // Add some selections + builder.addSelection(createSimpleAttributeExpression("Trace.transaction_name")); + builder.addSelection(createSimpleAttributeExpression("Trace.id")); + + // A function added into selections list is treated as a selection + Function avg = + Function.newBuilder() + .setFunctionName("AVG") + .setAlias("myAvgAlias") + .addArguments(createSimpleAttributeExpression("Trace.duration")) + .build(); + builder.addSelection(Expression.newBuilder().setFunction(avg)); + + // Add some group bys + builder.addGroupBy(createSimpleAttributeExpression("Trace.api_name")); + builder.addGroupBy(createSimpleAttributeExpression("Trace.service_name")); + QueryRequest queryRequest = builder.build(); + + ExecutionContext context = new ExecutionContext("test", queryRequest); + + // The order in resultSetMetadata.getColumnMetadataList() and selections is group bys, + // selections then aggregations + ResultSetMetadata resultSetMetadata = context.getResultSetMetadata(); + + assertNotNull(resultSetMetadata); + assertEquals(7, resultSetMetadata.getColumnMetadataCount()); + assertEquals("Trace.api_name", resultSetMetadata.getColumnMetadata(0).getColumnName()); + assertEquals("Trace.service_name", resultSetMetadata.getColumnMetadata(1).getColumnName()); + assertEquals("Trace.transaction_name", resultSetMetadata.getColumnMetadata(2).getColumnName()); + assertEquals("Trace.id", resultSetMetadata.getColumnMetadata(3).getColumnName()); + assertEquals("myAvgAlias", resultSetMetadata.getColumnMetadata(4).getColumnName()); + assertEquals("myCountAlias", resultSetMetadata.getColumnMetadata(5).getColumnName()); + assertEquals("MIN", resultSetMetadata.getColumnMetadata(6).getColumnName()); + + // Selections should correspond in size and order to the + // resultSetMetadata.getColumnMetadataList() + assertEquals(7, context.getAllSelections().size()); + Iterator selectionsIterator = context.getAllSelections().iterator(); + assertEquals( + createSimpleAttributeExpression("Trace.api_name").build(), selectionsIterator.next()); + assertEquals( + createSimpleAttributeExpression("Trace.service_name").build(), selectionsIterator.next()); + assertEquals( + createSimpleAttributeExpression("Trace.transaction_name").build(), + selectionsIterator.next()); + assertEquals(createSimpleAttributeExpression("Trace.id").build(), selectionsIterator.next()); + assertEquals(Expression.newBuilder().setFunction(avg).build(), selectionsIterator.next()); + assertEquals(Expression.newBuilder().setFunction(count).build(), selectionsIterator.next()); + assertEquals( + Expression.newBuilder().setFunction(minFunction).build(), selectionsIterator.next()); + } + @Test public void testSetTimeSeriesPeriod() { From 5ed48e40f968bb352422ef5cce658dfd57cb094b Mon Sep 17 00:00:00 2001 From: Sarthak Singhal Date: Mon, 20 Dec 2021 11:57:51 +0530 Subject: [PATCH 06/23] added unit test in pinot based rqst handler --- .../service/QueryRequestBuilderUtils.java | 7 + .../pinot/PinotBasedRequestHandlerTest.java | 289 ++++++++++++++++++ 2 files changed, 296 insertions(+) diff --git a/query-service-impl/src/test/java/org/hypertrace/core/query/service/QueryRequestBuilderUtils.java b/query-service-impl/src/test/java/org/hypertrace/core/query/service/QueryRequestBuilderUtils.java index 7d90b323..51b739e1 100644 --- a/query-service-impl/src/test/java/org/hypertrace/core/query/service/QueryRequestBuilderUtils.java +++ b/query-service-impl/src/test/java/org/hypertrace/core/query/service/QueryRequestBuilderUtils.java @@ -157,6 +157,13 @@ public static Filter createSimpleAttributeFilter( return createFilter(createSimpleAttributeExpression(column).build(), operator, expression); } + public static Filter createSimpleAttributeFilter(String column, Operator operator, String value) { + return createFilter( + createSimpleAttributeExpression(column).build(), + operator, + createStringLiteralValueExpression(value)); + } + public static Filter createFilter( Expression columnExpression, Operator operator, Expression expression) { return Filter.newBuilder() diff --git a/query-service-impl/src/test/java/org/hypertrace/core/query/service/pinot/PinotBasedRequestHandlerTest.java b/query-service-impl/src/test/java/org/hypertrace/core/query/service/pinot/PinotBasedRequestHandlerTest.java index 54845bd7..a6bb46ae 100644 --- a/query-service-impl/src/test/java/org/hypertrace/core/query/service/pinot/PinotBasedRequestHandlerTest.java +++ b/query-service-impl/src/test/java/org/hypertrace/core/query/service/pinot/PinotBasedRequestHandlerTest.java @@ -1,6 +1,10 @@ package org.hypertrace.core.query.service.pinot; import static org.hypertrace.core.query.service.QueryRequestBuilderUtils.createComplexAttributeExpression; +import static org.hypertrace.core.query.service.QueryRequestBuilderUtils.createIntLiteralValueExpression; +import static org.hypertrace.core.query.service.QueryRequestBuilderUtils.createLongLiteralValueExpression; +import static org.hypertrace.core.query.service.QueryRequestUtil.createBooleanLiteralExpression; +import static org.hypertrace.core.query.service.QueryRequestUtil.createSimpleAttributeExpression; import static org.mockito.ArgumentMatchers.any; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; @@ -466,6 +470,236 @@ public void testCanHandleWithEqViewFilter() { } } + @Test + public void testCanHandleWithMultipleViewFiltersWithAttributeExpression() { + for (Config config : serviceConfig.getConfigList("queryRequestHandlersConfig")) { + if (!isPinotConfig(config)) { + continue; + } + + PinotBasedRequestHandler handler = + new PinotBasedRequestHandler( + config.getString("name"), config.getConfig("requestHandlerInfo")); + + // Verify that the entry span view handler can handle the query which has the filter + // on the column which has a view filter. + if (config.getString("name").equals("error-entry-span-view-handler")) { + // Positive case, straight forward. + QueryRequest request = + QueryRequest.newBuilder() + .setDistinctSelections(true) + .addSelection(createSimpleAttributeExpression("EVENT.startTime")) + .addSelection(createSimpleAttributeExpression("EVENT.id")) + .addSelection(createSimpleAttributeExpression("EVENT.traceId")) + .setFilter( + Filter.newBuilder() + .setOperator(Operator.AND) + .addChildFilter( + QueryRequestBuilderUtils.createSimpleAttributeFilter( + "EVENT.isEntrySpan", Operator.EQ, "true")) + .addChildFilter( + QueryRequestBuilderUtils.createSimpleAttributeFilter( + "EVENT.statusCode", Operator.EQ, "401"))) + .build(); + + ExecutionContext executionContext = new ExecutionContext("__default", request); + QueryCost cost = handler.canHandle(request, executionContext); + Assertions.assertTrue(cost.getCost() >= 0.0d && cost.getCost() < 1.0d); + + // Positive case but the filters are AND'ed in two different child filters. + Filter filter = + Filter.newBuilder() + .setOperator(Operator.AND) + .addChildFilter( + QueryRequestBuilderUtils.createSimpleAttributeFilter( + "EVENT.isEntrySpan", Operator.EQ, "true")) + .addChildFilter( + QueryRequestBuilderUtils.createSimpleAttributeFilter( + "EVENT.startTime", + Operator.GT, + QueryRequestBuilderUtils.createLongLiteralValueExpression( + System.currentTimeMillis()))) + .build(); + request = + QueryRequest.newBuilder() + .setDistinctSelections(true) + .addSelection(createSimpleAttributeExpression("EVENT.startTime")) + .addSelection(createSimpleAttributeExpression("EVENT.id")) + .addSelection(createSimpleAttributeExpression("EVENT.traceId")) + .setFilter( + Filter.newBuilder() + .setOperator(Operator.AND) + .addChildFilter(filter) + .addChildFilter( + QueryRequestBuilderUtils.createSimpleAttributeFilter( + "EVENT.statusCode", + Operator.EQ, + createIntLiteralValueExpression(401)))) + .build(); + + executionContext = new ExecutionContext("__default", request); + + cost = handler.canHandle(request, executionContext); + Assertions.assertTrue(cost.getCost() >= 0.0d && cost.getCost() < 1.0d); + + // Negative case. Query has only one leaf filter and it matches only one of the view + // filters + request = + QueryRequest.newBuilder() + .setDistinctSelections(true) + .addSelection(createSimpleAttributeExpression("EVENT.startTime")) + .addSelection(createSimpleAttributeExpression("EVENT.id")) + .addSelection(createSimpleAttributeExpression("EVENT.traceId")) + .setFilter( + QueryRequestBuilderUtils.createSimpleAttributeFilter( + "EVENT.isEntrySpan", Operator.EQ, createBooleanLiteralExpression(true))) + .build(); + + executionContext = new ExecutionContext("__default", request); + cost = handler.canHandle(request, executionContext); + Assertions.assertFalse(cost.getCost() >= 0.0d && cost.getCost() < 1.0d); + + // Negative case. Only one view filter is present in the query filters + request = + QueryRequest.newBuilder() + .setDistinctSelections(true) + .addSelection(createSimpleAttributeExpression("EVENT.startTime")) + .addSelection(createSimpleAttributeExpression("EVENT.id")) + .addSelection(createSimpleAttributeExpression("EVENT.traceId")) + .setFilter( + Filter.newBuilder() + .setOperator(Operator.AND) + .addChildFilter( + QueryRequestBuilderUtils.createSimpleAttributeFilter( + "EVENT.isEntrySpan", + Operator.EQ, + createBooleanLiteralExpression(true))) + .addChildFilter( + QueryRequestBuilderUtils.createSimpleAttributeFilter( + "EVENT.startTime", + Operator.GT, + QueryRequestBuilderUtils.createLongLiteralValueExpression( + System.currentTimeMillis()))) + .build()) + .build(); + + executionContext = new ExecutionContext("__default", request); + cost = handler.canHandle(request, executionContext); + Assertions.assertFalse(cost.getCost() >= 0.0d && cost.getCost() < 1.0d); + + // Negative case with correct filters but 'OR' operation. + request = + QueryRequest.newBuilder() + .setDistinctSelections(true) + .addSelection(createSimpleAttributeExpression("EVENT.startTime")) + .addSelection(createSimpleAttributeExpression("EVENT.id")) + .addSelection(createSimpleAttributeExpression("EVENT.traceId")) + .setFilter( + Filter.newBuilder() + .setOperator(Operator.OR) + .addChildFilter( + QueryRequestBuilderUtils.createSimpleAttributeFilter( + "EVENT.isEntrySpan", + Operator.EQ, + createBooleanLiteralExpression(true))) + .addChildFilter( + QueryRequestBuilderUtils.createSimpleAttributeFilter( + "EVENT.statusCode", + Operator.EQ, + createLongLiteralValueExpression(401L)))) + .build(); + + executionContext = new ExecutionContext("__default", request); + cost = handler.canHandle(request, executionContext); + Assertions.assertFalse(cost.getCost() >= 0.0d && cost.getCost() < 1.0d); + + // Negative case with a complex filter but 'OR' at the root level, hence shouldn't match. + filter = + Filter.newBuilder() + .setOperator(Operator.AND) + .addChildFilter( + QueryRequestBuilderUtils.createSimpleAttributeFilter( + "EVENT.isEntrySpan", Operator.EQ, createBooleanLiteralExpression(true))) + .addChildFilter( + QueryRequestBuilderUtils.createSimpleAttributeFilter( + "EVENT.statusCode", Operator.EQ, "401")) + .build(); + request = + QueryRequest.newBuilder() + .setDistinctSelections(true) + .addSelection(createSimpleAttributeExpression("EVENT.startTime")) + .addSelection(createSimpleAttributeExpression("EVENT.id")) + .addSelection(createSimpleAttributeExpression("EVENT.traceId")) + .setFilter( + Filter.newBuilder() + .setOperator(Operator.OR) + .addChildFilter( + QueryRequestBuilderUtils.createSimpleAttributeFilter( + "EVENT.isEntrySpan", + Operator.EQ, + createBooleanLiteralExpression(true))) + .addChildFilter(filter)) + .build(); + + executionContext = new ExecutionContext("__default", request); + cost = handler.canHandle(request, executionContext); + Assertions.assertFalse(cost.getCost() >= 0.0d && cost.getCost() < 1.0d); + + // Negative case. Value in query filter is different from the value in view filter + request = + QueryRequest.newBuilder() + .setDistinctSelections(true) + .addSelection(createSimpleAttributeExpression("EVENT.startTime")) + .addSelection(createSimpleAttributeExpression("EVENT.id")) + .addSelection(createSimpleAttributeExpression("EVENT.traceId")) + .setFilter( + Filter.newBuilder() + .setOperator(Operator.AND) + .addChildFilter( + QueryRequestBuilderUtils.createSimpleAttributeFilter( + "EVENT.isEntrySpan", Operator.EQ, "true")) + .addChildFilter( + QueryRequestBuilderUtils.createSimpleAttributeFilter( + "EVENT.statusCode", Operator.EQ, "200"))) + .build(); + + executionContext = new ExecutionContext("__default", request); + cost = handler.canHandle(request, executionContext); + Assertions.assertFalse(cost.getCost() >= 0.0d && cost.getCost() < 1.0d); + + // Negative case. Unsupported operator in the query filter. + request = + QueryRequest.newBuilder() + .setDistinctSelections(true) + .addSelection(createSimpleAttributeExpression("EVENT.startTime")) + .addSelection(createSimpleAttributeExpression("EVENT.id")) + .addSelection(createSimpleAttributeExpression("EVENT.traceId")) + .setFilter( + QueryRequestBuilderUtils.createSimpleAttributeFilter( + "EVENT.isEntrySpan", + Operator.IN, + QueryRequestUtil.createStringLiteralValueExpression("dummy"))) + .build(); + + executionContext = new ExecutionContext("__default", request); + cost = handler.canHandle(request, executionContext); + Assertions.assertFalse(cost.getCost() >= 0.0d && cost.getCost() < 1.0d); + + // Negative case. Any query without filter should not be handled. + request = + QueryRequest.newBuilder() + .setDistinctSelections(true) + .addSelection(createSimpleAttributeExpression("EVENT.startTime")) + .addSelection(createSimpleAttributeExpression("EVENT.id")) + .addSelection(createSimpleAttributeExpression("EVENT.traceId")) + .build(); + executionContext = new ExecutionContext("__default", request); + QueryCost negativeCost = handler.canHandle(request, executionContext); + Assertions.assertFalse(negativeCost.getCost() >= 0.0d && negativeCost.getCost() < 1.0d); + } + } + } + @Test public void testCanHandleWithMultipleViewFilters() { for (Config config : serviceConfig.getConfigList("queryRequestHandlersConfig")) { @@ -1215,6 +1449,61 @@ public void testNullTenantIdQueryRequestContextThrowsNPE() { .blockingSubscribe()); } + @Test + public void + testGroupBysAndAggregationsMixedWithSelectionsThrowsExceptionWhenDistinctSelectionIsSpecifiedWithAttributeExpression() { + // Setting distinct selections and mixing selections and group bys should throw exception + QueryRequest request = + QueryRequest.newBuilder() + .setDistinctSelections(true) + .addSelection(createSimpleAttributeExpression("col1")) + .addSelection(createSimpleAttributeExpression("col2")) + .addGroupBy(createSimpleAttributeExpression("col3")) + .build(); + Assertions.assertThrows( + IllegalArgumentException.class, + () -> + pinotBasedRequestHandler + .handleRequest(request, new ExecutionContext("test-tenant-id", request)) + .blockingSubscribe()); + + // Setting distinct selections and mixing selections and aggregations should throw exception + QueryRequest request2 = + QueryRequest.newBuilder() + .setDistinctSelections(true) + .addSelection(createSimpleAttributeExpression("col1")) + .addSelection(createSimpleAttributeExpression("col2")) + .addAggregation( + QueryRequestBuilderUtils.createAliasedFunctionExpressionWithSimpleAttribute( + "AVG", "duration", "avg_duration")) + .build(); + Assertions.assertThrows( + IllegalArgumentException.class, + () -> + pinotBasedRequestHandler + .handleRequest(request2, new ExecutionContext("test-tenant-id", request2)) + .blockingSubscribe()); + + // Setting distinct selections and mixing selections, group bys and aggregations should throw + // exception + QueryRequest request3 = + QueryRequest.newBuilder() + .setDistinctSelections(true) + .addSelection(createSimpleAttributeExpression("col1")) + .addSelection(createSimpleAttributeExpression("col2")) + .addGroupBy(createSimpleAttributeExpression("col3")) + .addAggregation( + QueryRequestBuilderUtils.createAliasedFunctionExpressionWithSimpleAttribute( + "AVG", "duration", "avg_duration")) + .build(); + Assertions.assertThrows( + IllegalArgumentException.class, + () -> + pinotBasedRequestHandler + .handleRequest(request3, new ExecutionContext("test-tenant-id", request3)) + .blockingSubscribe()); + } + @Test public void testWithMockPinotClient() throws IOException { for (Config config : serviceConfig.getConfigList("queryRequestHandlersConfig")) { From 023340180bfa20c8ba636c1aeb1ee3b3594425bb Mon Sep 17 00:00:00 2001 From: Sarthak Singhal Date: Tue, 21 Dec 2021 15:14:13 +0530 Subject: [PATCH 07/23] changed function signature --- .../core/query/service/ExecutionContext.java | 16 +++++-- .../core/query/service/QueryRequestUtil.java | 46 ++++++++----------- .../pinot/PinotBasedRequestHandler.java | 16 +++---- .../QueryRequestToPinotSQLConverter.java | 17 +++++-- .../prometheus/FilterToPromqlConverter.java | 3 +- .../PrometheusBasedRequestHandler.java | 3 +- .../service/prometheus/PrometheusUtils.java | 3 +- .../QueryRequestEligibilityValidator.java | 6 ++- .../QueryRequestToPromqlConverter.java | 5 +- 9 files changed, 63 insertions(+), 52 deletions(-) diff --git a/query-service-impl/src/main/java/org/hypertrace/core/query/service/ExecutionContext.java b/query-service-impl/src/main/java/org/hypertrace/core/query/service/ExecutionContext.java index b6cd5b27..d25caf8d 100644 --- a/query-service-impl/src/main/java/org/hypertrace/core/query/service/ExecutionContext.java +++ b/query-service-impl/src/main/java/org/hypertrace/core/query/service/ExecutionContext.java @@ -2,6 +2,9 @@ import static org.hypertrace.core.query.service.QueryRequestUtil.getAlias; import static org.hypertrace.core.query.service.QueryRequestUtil.getLogicalColumnName; +import static org.hypertrace.core.query.service.api.Expression.ValueCase.ATTRIBUTE_EXPRESSION; +import static org.hypertrace.core.query.service.api.Expression.ValueCase.COLUMNIDENTIFIER; +import static org.hypertrace.core.query.service.api.Expression.ValueCase.FUNCTION; import com.google.common.base.Supplier; import com.google.common.base.Suppliers; @@ -141,7 +144,8 @@ private ColumnMetadata toColumnMetadata(Expression expression) { case COLUMNIDENTIFIER: case ATTRIBUTE_EXPRESSION: case FUNCTION: - builder.setColumnName(getAlias(expression)); + String alias = getAlias(expression).orElseThrow(IllegalArgumentException::new); + builder.setColumnName(alias); builder.setValueType(ValueType.STRING); builder.setIsRepeated(false); break; @@ -158,7 +162,9 @@ private void extractColumns(List columns, Expression expression) { switch (valueCase) { case COLUMNIDENTIFIER: case ATTRIBUTE_EXPRESSION: - columns.add(getLogicalColumnName(expression)); + String logicalColumnName = + getLogicalColumnName(expression).orElseThrow(IllegalArgumentException::new); + columns.add(logicalColumnName); break; case LITERAL: // no columns @@ -219,12 +225,12 @@ private Optional buildQueryTimeRange(Filter filter, String timeF private boolean isMatchingFilter(Filter filter, String column, Collection operators) { - // if not leaf filter, then return false - if (!filter.hasLhs()) { + Optional logicalColumnName = getLogicalColumnName(filter.getLhs()); + if (logicalColumnName.isEmpty()) { return false; } - return column.equals(getLogicalColumnName(filter.getLhs())) + return column.equals(logicalColumnName.get()) && (operators.stream() .anyMatch(operator -> Objects.equals(operator, filter.getOperator()))); } diff --git a/query-service-impl/src/main/java/org/hypertrace/core/query/service/QueryRequestUtil.java b/query-service-impl/src/main/java/org/hypertrace/core/query/service/QueryRequestUtil.java index 1518b170..f63db9ee 100644 --- a/query-service-impl/src/main/java/org/hypertrace/core/query/service/QueryRequestUtil.java +++ b/query-service-impl/src/main/java/org/hypertrace/core/query/service/QueryRequestUtil.java @@ -4,6 +4,7 @@ import static org.hypertrace.core.query.service.api.Expression.ValueCase.COLUMNIDENTIFIER; import static org.hypertrace.core.query.service.api.Expression.ValueCase.FUNCTION; +import java.util.Optional; import org.hypertrace.core.query.service.api.AttributeExpression; import org.hypertrace.core.query.service.api.ColumnIdentifier; import org.hypertrace.core.query.service.api.Expression; @@ -125,47 +126,38 @@ public static boolean isDateTimeFunction(Expression expression) { && expression.getFunction().getFunctionName().equals("dateTimeConvert"); } - public static String getLogicalColumnName(Expression expression) { + public static Optional getLogicalColumnName(Expression expression) { switch (expression.getValueCase()) { case COLUMNIDENTIFIER: - return expression.getColumnIdentifier().getColumnName(); + return Optional.of(expression.getColumnIdentifier().getColumnName()); case ATTRIBUTE_EXPRESSION: - return expression.getAttributeExpression().getAttributeId(); + return Optional.of(expression.getAttributeExpression().getAttributeId()); default: - throw new IllegalArgumentException( - "Supports " - + ATTRIBUTE_EXPRESSION - + " and " - + COLUMNIDENTIFIER - + " expression type only"); + return Optional.empty(); } } - public static String getAlias(Expression expression) { + public static Optional getAlias(Expression expression) { switch (expression.getValueCase()) { case COLUMNIDENTIFIER: - return expression.getColumnIdentifier().getAlias().isBlank() - ? getLogicalColumnName(expression) - : expression.getColumnIdentifier().getAlias(); + return Optional.of( + expression.getColumnIdentifier().getAlias().isBlank() + ? getLogicalColumnName(expression).get() + : expression.getColumnIdentifier().getAlias()); case ATTRIBUTE_EXPRESSION: - return expression.getAttributeExpression().getAlias().isBlank() - ? getLogicalColumnName(expression) - : expression.getAttributeExpression().getAlias(); + return Optional.of( + expression.getAttributeExpression().getAlias().isBlank() + ? getLogicalColumnName(expression).get() + : expression.getAttributeExpression().getAlias()); case FUNCTION: // todo: handle recursive functions max(rollup(time,50) // workaround is to use alias for now - return expression.getFunction().getAlias().isBlank() - ? expression.getFunction().getFunctionName() - : expression.getFunction().getAlias(); + return Optional.of( + expression.getFunction().getAlias().isBlank() + ? expression.getFunction().getFunctionName() + : expression.getFunction().getAlias()); default: - throw new IllegalArgumentException( - "Supports " - + ATTRIBUTE_EXPRESSION - + " , " - + FUNCTION - + " and " - + COLUMNIDENTIFIER - + " expression type only"); + return Optional.empty(); } } } diff --git a/query-service-impl/src/main/java/org/hypertrace/core/query/service/pinot/PinotBasedRequestHandler.java b/query-service-impl/src/main/java/org/hypertrace/core/query/service/pinot/PinotBasedRequestHandler.java index 2c40f9a3..98f3fecf 100644 --- a/query-service-impl/src/main/java/org/hypertrace/core/query/service/pinot/PinotBasedRequestHandler.java +++ b/query-service-impl/src/main/java/org/hypertrace/core/query/service/pinot/PinotBasedRequestHandler.java @@ -226,12 +226,8 @@ && rhsHasLongValue(filter.getRhs())) { } private boolean lhsIsStartTimeAttribute(Expression lhs) { - if (lhs.hasColumnIdentifier() || lhs.hasAttributeExpression()) { - return startTimeAttributeName - .map(attributeName -> attributeName.equals(getLogicalColumnName(lhs))) - .orElse(false); - } - return false; + return startTimeAttributeName.isPresent() + && startTimeAttributeName.equals(getLogicalColumnName(lhs)); } private boolean rhsHasLongValue(Expression rhs) { @@ -248,7 +244,7 @@ private Set getMatchingViewFilterColumns( // return it. if (filter.getChildFilterCount() == 0) { return doesSingleViewFilterMatchLeafQueryFilter(viewFilterMap, filter) - ? Set.of(getLogicalColumnName(filter.getLhs())) + ? Set.of(getLogicalColumnName(filter.getLhs()).orElseThrow(IllegalArgumentException::new)) : Set.of(); } else { // 2. Internal filter node. Recursively get the matching nodes from children. @@ -285,7 +281,8 @@ private boolean doesSingleViewFilterMatchLeafQueryFilter( return false; } - String columnName = getLogicalColumnName(queryFilter.getLhs()); + String columnName = + getLogicalColumnName(queryFilter.getLhs()).orElseThrow(IllegalArgumentException::new); ViewColumnFilter viewColumnFilter = viewFilterMap.get(columnName); if (viewColumnFilter == null) { return false; @@ -473,7 +470,8 @@ private Filter removeViewColumnFilter( private Filter rewriteLeafFilter( Filter queryFilter, Map columnFilterMap) { ViewColumnFilter viewColumnFilter = - columnFilterMap.get(getLogicalColumnName(queryFilter.getLhs())); + columnFilterMap.get( + getLogicalColumnName(queryFilter.getLhs()).orElseThrow(IllegalArgumentException::new)); // If the RHS of both the view filter and query filter match, return empty filter. if (viewColumnFilter != null && isEquals(viewColumnFilter.getValues(), queryFilter.getRhs())) { return Filter.getDefaultInstance(); diff --git a/query-service-impl/src/main/java/org/hypertrace/core/query/service/pinot/QueryRequestToPinotSQLConverter.java b/query-service-impl/src/main/java/org/hypertrace/core/query/service/pinot/QueryRequestToPinotSQLConverter.java index b405a901..8b219716 100644 --- a/query-service-impl/src/main/java/org/hypertrace/core/query/service/pinot/QueryRequestToPinotSQLConverter.java +++ b/query-service-impl/src/main/java/org/hypertrace/core/query/service/pinot/QueryRequestToPinotSQLConverter.java @@ -211,7 +211,7 @@ private Expression handleValueConversionForLiteralExpression(Expression lhs, Exp return rhs; } - String lhsColumnName = getLogicalColumnName(lhs); + String lhsColumnName = getLogicalColumnName(lhs).orElseThrow(IllegalArgumentException::new); try { Value value = DestinationColumnValueConverter.INSTANCE.convert( @@ -272,7 +272,8 @@ private String convertExpressionToString( case COLUMNIDENTIFIER: // this takes care of the Map Type where it's split into 2 columns List columnNames = - viewDefinition.getPhysicalColumnNames(getLogicalColumnName(expression)); + viewDefinition.getPhysicalColumnNames( + getLogicalColumnName(expression).orElseThrow(IllegalArgumentException::new)); return joiner.join(columnNames); case ATTRIBUTE_EXPRESSION: if (isAttributeExpressionWithSubpath(expression)) { @@ -292,7 +293,9 @@ private String convertExpressionToString( valCol); } else { // this takes care of the Map Type where it's split into 2 columns - columnNames = viewDefinition.getPhysicalColumnNames(getLogicalColumnName(expression)); + columnNames = + viewDefinition.getPhysicalColumnNames( + getLogicalColumnName(expression).orElseThrow(IllegalArgumentException::new)); return joiner.join(columnNames); } case LITERAL: @@ -313,7 +316,9 @@ private String convertExpressionToString( } private String convertExpressionToMapKeyColumn(Expression expression) { - String col = viewDefinition.getKeyColumnNameForMap(getLogicalColumnName(expression)); + String col = + viewDefinition.getKeyColumnNameForMap( + getLogicalColumnName(expression).orElseThrow(IllegalArgumentException::new)); if (col != null && col.length() > 0) { return col; } @@ -321,7 +326,9 @@ private String convertExpressionToMapKeyColumn(Expression expression) { } private String convertExpressionToMapValueColumn(Expression expression) { - String col = viewDefinition.getValueColumnNameForMap(getLogicalColumnName(expression)); + String col = + viewDefinition.getValueColumnNameForMap( + getLogicalColumnName(expression).orElseThrow(IllegalArgumentException::new)); if (col != null && col.length() > 0) { return col; } diff --git a/query-service-impl/src/main/java/org/hypertrace/core/query/service/prometheus/FilterToPromqlConverter.java b/query-service-impl/src/main/java/org/hypertrace/core/query/service/prometheus/FilterToPromqlConverter.java index b596d652..6cb99fc3 100644 --- a/query-service-impl/src/main/java/org/hypertrace/core/query/service/prometheus/FilterToPromqlConverter.java +++ b/query-service-impl/src/main/java/org/hypertrace/core/query/service/prometheus/FilterToPromqlConverter.java @@ -28,7 +28,8 @@ void convertFilterToString( } } else { if (QueryRequestUtil.isSimpleAttributeExpression(filter.getLhs()) - && timeFilterColumn.equals(getLogicalColumnName(filter.getLhs()))) { + && timeFilterColumn.equals( + getLogicalColumnName(filter.getLhs()).orElseThrow(IllegalArgumentException::new))) { return; } StringBuilder builder = new StringBuilder(); diff --git a/query-service-impl/src/main/java/org/hypertrace/core/query/service/prometheus/PrometheusBasedRequestHandler.java b/query-service-impl/src/main/java/org/hypertrace/core/query/service/prometheus/PrometheusBasedRequestHandler.java index a4320e8a..d2522a5a 100644 --- a/query-service-impl/src/main/java/org/hypertrace/core/query/service/prometheus/PrometheusBasedRequestHandler.java +++ b/query-service-impl/src/main/java/org/hypertrace/core/query/service/prometheus/PrometheusBasedRequestHandler.java @@ -138,7 +138,8 @@ private List prepareSelectionColumnSet( switch (valueCase) { case ATTRIBUTE_EXPRESSION: case COLUMNIDENTIFIER: - return QueryRequestUtil.getLogicalColumnName(expression); + return QueryRequestUtil.getLogicalColumnName(expression) + .orElseThrow(IllegalArgumentException::new); case FUNCTION: if (QueryRequestUtil.isDateTimeFunction(expression)) { return executionContext.getTimeFilterColumn(); diff --git a/query-service-impl/src/main/java/org/hypertrace/core/query/service/prometheus/PrometheusUtils.java b/query-service-impl/src/main/java/org/hypertrace/core/query/service/prometheus/PrometheusUtils.java index 1f231cfb..fb3053f1 100644 --- a/query-service-impl/src/main/java/org/hypertrace/core/query/service/prometheus/PrometheusUtils.java +++ b/query-service-impl/src/main/java/org/hypertrace/core/query/service/prometheus/PrometheusUtils.java @@ -7,7 +7,8 @@ class PrometheusUtils { static String generateAliasForMetricFunction(Expression functionExpression) { String functionName = functionExpression.getFunction().getFunctionName().toUpperCase(); String columnName = - QueryRequestUtil.getLogicalColumnName(functionExpression.getFunction().getArguments(0)); + QueryRequestUtil.getLogicalColumnName(functionExpression.getFunction().getArguments(0)) + .orElseThrow(IllegalArgumentException::new); return String.join(":", functionName, columnName); } } diff --git a/query-service-impl/src/main/java/org/hypertrace/core/query/service/prometheus/QueryRequestEligibilityValidator.java b/query-service-impl/src/main/java/org/hypertrace/core/query/service/prometheus/QueryRequestEligibilityValidator.java index 2968d711..4473be8a 100644 --- a/query-service-impl/src/main/java/org/hypertrace/core/query/service/prometheus/QueryRequestEligibilityValidator.java +++ b/query-service-impl/src/main/java/org/hypertrace/core/query/service/prometheus/QueryRequestEligibilityValidator.java @@ -4,6 +4,7 @@ import com.google.common.base.Preconditions; import java.util.List; +import java.util.Optional; import java.util.Set; import java.util.function.Predicate; import java.util.stream.Collectors; @@ -89,12 +90,14 @@ private boolean selectionAndGroupByOnDifferentColumn( Set selections = selectionList.stream() .map(QueryRequestUtil::getLogicalColumnName) + .map(Optional::get) .collect(Collectors.toSet()); Set groupBys = groupByList.stream() .filter(Predicate.not(QueryRequestUtil::isDateTimeFunction)) .map(QueryRequestUtil::getLogicalColumnName) + .map(Optional::get) .collect(Collectors.toSet()); return !selections.equals(groupBys); } @@ -113,7 +116,8 @@ private boolean areAggregationsNotSupported(List aggregationList) { return true; } Expression functionArgument = function.getArgumentsList().get(0); - String attributeId = getLogicalColumnName(functionArgument); + String attributeId = + getLogicalColumnName(functionArgument).orElseThrow(IllegalArgumentException::new); if (!PrometheusFunctionConverter.supportedFunctions.contains( function.getFunctionName())) { return true; diff --git a/query-service-impl/src/main/java/org/hypertrace/core/query/service/prometheus/QueryRequestToPromqlConverter.java b/query-service-impl/src/main/java/org/hypertrace/core/query/service/prometheus/QueryRequestToPromqlConverter.java index 8a833e8d..d1ff8c35 100644 --- a/query-service-impl/src/main/java/org/hypertrace/core/query/service/prometheus/QueryRequestToPromqlConverter.java +++ b/query-service-impl/src/main/java/org/hypertrace/core/query/service/prometheus/QueryRequestToPromqlConverter.java @@ -157,11 +157,12 @@ private String buildQuery( private MetricConfig getMetricConfigForFunction(Expression functionSelection) { return prometheusViewDefinition.getMetricConfigForLogicalMetricName( - getLogicalColumnName(functionSelection.getFunction().getArgumentsList().get(0))); + getLogicalColumnName(functionSelection.getFunction().getArgumentsList().get(0)) + .orElseThrow(IllegalArgumentException::new)); } private String convertColumnAttributeToString(Expression expression) { return prometheusViewDefinition.getPhysicalColumnNameForLogicalColumnName( - getLogicalColumnName(expression)); + getLogicalColumnName(expression).orElseThrow(IllegalArgumentException::new)); } } From 8f251196af6599a73ea500c622a5aa1f03209ba6 Mon Sep 17 00:00:00 2001 From: Sarthak Singhal Date: Tue, 21 Dec 2021 15:17:42 +0530 Subject: [PATCH 08/23] resolved comments --- .../service/pinot/PinotBasedRequestHandler.java | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/query-service-impl/src/main/java/org/hypertrace/core/query/service/pinot/PinotBasedRequestHandler.java b/query-service-impl/src/main/java/org/hypertrace/core/query/service/pinot/PinotBasedRequestHandler.java index 98f3fecf..0170a0f4 100644 --- a/query-service-impl/src/main/java/org/hypertrace/core/query/service/pinot/PinotBasedRequestHandler.java +++ b/query-service-impl/src/main/java/org/hypertrace/core/query/service/pinot/PinotBasedRequestHandler.java @@ -273,17 +273,17 @@ private Set getMatchingViewFilterColumns( */ private boolean doesSingleViewFilterMatchLeafQueryFilter( Map viewFilterMap, Filter queryFilter) { - if (queryFilter.getLhs().getValueCase() != ValueCase.COLUMNIDENTIFIER - && queryFilter.getLhs().getValueCase() != ValueCase.ATTRIBUTE_EXPRESSION) { + + if (queryFilter.getOperator() != Operator.IN && queryFilter.getOperator() != Operator.EQ) { return false; } - if (queryFilter.getOperator() != Operator.IN && queryFilter.getOperator() != Operator.EQ) { + + Optional columnName = getLogicalColumnName(queryFilter.getLhs()); + if (columnName.isEmpty()) { return false; } - String columnName = - getLogicalColumnName(queryFilter.getLhs()).orElseThrow(IllegalArgumentException::new); - ViewColumnFilter viewColumnFilter = viewFilterMap.get(columnName); + ViewColumnFilter viewColumnFilter = viewFilterMap.get(columnName.get()); if (viewColumnFilter == null) { return false; } From a6137921179d4d497c594379805f049a30248de1 Mon Sep 17 00:00:00 2001 From: Sarthak Singhal Date: Tue, 21 Dec 2021 21:23:37 +0530 Subject: [PATCH 09/23] nit --- .../hypertrace/core/query/service/ExecutionContext.java | 8 +------- .../query/service/pinot/PinotBasedRequestHandler.java | 8 ++------ .../prometheus/QueryRequestEligibilityValidator.java | 4 ++-- 3 files changed, 5 insertions(+), 15 deletions(-) diff --git a/query-service-impl/src/main/java/org/hypertrace/core/query/service/ExecutionContext.java b/query-service-impl/src/main/java/org/hypertrace/core/query/service/ExecutionContext.java index d25caf8d..49761ca2 100644 --- a/query-service-impl/src/main/java/org/hypertrace/core/query/service/ExecutionContext.java +++ b/query-service-impl/src/main/java/org/hypertrace/core/query/service/ExecutionContext.java @@ -224,13 +224,7 @@ private Optional buildQueryTimeRange(Filter filter, String timeF } private boolean isMatchingFilter(Filter filter, String column, Collection operators) { - - Optional logicalColumnName = getLogicalColumnName(filter.getLhs()); - if (logicalColumnName.isEmpty()) { - return false; - } - - return column.equals(logicalColumnName.get()) + return getLogicalColumnName(filter.getLhs()).map(column::equals).orElse(false) && (operators.stream() .anyMatch(operator -> Objects.equals(operator, filter.getOperator()))); } diff --git a/query-service-impl/src/main/java/org/hypertrace/core/query/service/pinot/PinotBasedRequestHandler.java b/query-service-impl/src/main/java/org/hypertrace/core/query/service/pinot/PinotBasedRequestHandler.java index 0170a0f4..12d37ed4 100644 --- a/query-service-impl/src/main/java/org/hypertrace/core/query/service/pinot/PinotBasedRequestHandler.java +++ b/query-service-impl/src/main/java/org/hypertrace/core/query/service/pinot/PinotBasedRequestHandler.java @@ -278,12 +278,8 @@ private boolean doesSingleViewFilterMatchLeafQueryFilter( return false; } - Optional columnName = getLogicalColumnName(queryFilter.getLhs()); - if (columnName.isEmpty()) { - return false; - } - - ViewColumnFilter viewColumnFilter = viewFilterMap.get(columnName.get()); + ViewColumnFilter viewColumnFilter = + viewFilterMap.get(getLogicalColumnName(queryFilter.getLhs()).orElse(null)); if (viewColumnFilter == null) { return false; } diff --git a/query-service-impl/src/main/java/org/hypertrace/core/query/service/prometheus/QueryRequestEligibilityValidator.java b/query-service-impl/src/main/java/org/hypertrace/core/query/service/prometheus/QueryRequestEligibilityValidator.java index 4473be8a..0ab9cf34 100644 --- a/query-service-impl/src/main/java/org/hypertrace/core/query/service/prometheus/QueryRequestEligibilityValidator.java +++ b/query-service-impl/src/main/java/org/hypertrace/core/query/service/prometheus/QueryRequestEligibilityValidator.java @@ -90,14 +90,14 @@ private boolean selectionAndGroupByOnDifferentColumn( Set selections = selectionList.stream() .map(QueryRequestUtil::getLogicalColumnName) - .map(Optional::get) + .map(Optional::orElseThrow) .collect(Collectors.toSet()); Set groupBys = groupByList.stream() .filter(Predicate.not(QueryRequestUtil::isDateTimeFunction)) .map(QueryRequestUtil::getLogicalColumnName) - .map(Optional::get) + .map(Optional::orElseThrow) .collect(Collectors.toSet()); return !selections.equals(groupBys); } From 4603ef816911e4f51703e449ed4c4193aa9f442f Mon Sep 17 00:00:00 2001 From: Sarthak Singhal Date: Wed, 22 Dec 2021 11:36:58 +0530 Subject: [PATCH 10/23] fixed unit tests --- .../service/projection/ProjectionTransformationTest.java | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/query-service-impl/src/test/java/org/hypertrace/core/query/service/projection/ProjectionTransformationTest.java b/query-service-impl/src/test/java/org/hypertrace/core/query/service/projection/ProjectionTransformationTest.java index 374a2811..a1654f3f 100644 --- a/query-service-impl/src/test/java/org/hypertrace/core/query/service/projection/ProjectionTransformationTest.java +++ b/query-service-impl/src/test/java/org/hypertrace/core/query/service/projection/ProjectionTransformationTest.java @@ -81,6 +81,7 @@ void beforeEach() { @Test void transQueryWithComplexAttributeExpression_SingleFilter() { this.mockAttribute("server", AttributeMetadata.getDefaultInstance()); + this.mockAttribute("Span.tags", AttributeMetadata.getDefaultInstance()); Expression spanTags = createComplexAttributeExpression("Span.tags", "span.kind").build(); Filter filter = @@ -112,6 +113,7 @@ void transQueryWithComplexAttributeExpression_SingleFilter() { void transQueryWithComplexAttributeExpression_MultipleFilter() { this.mockAttribute("server", AttributeMetadata.getDefaultInstance()); this.mockAttribute("0", AttributeMetadata.getDefaultInstance()); + this.mockAttribute("Span.tags", AttributeMetadata.getDefaultInstance()); Expression spanTags1 = createComplexAttributeExpression("Span.tags", "FLAGS").build(); Expression spanTags2 = createComplexAttributeExpression("Span.tags", "span.kind").build(); @@ -155,6 +157,7 @@ void transQueryWithComplexAttributeExpression_HierarchicalFilter() { this.mockAttribute("server", AttributeMetadata.getDefaultInstance()); this.mockAttribute("0", AttributeMetadata.getDefaultInstance()); this.mockAttribute(SIMPLE_ATTRIBUTE_ID, AttributeMetadata.getDefaultInstance()); + this.mockAttribute("Span.tags", AttributeMetadata.getDefaultInstance()); Expression spanTags1 = createComplexAttributeExpression("Span.tags", "FLAGS").build(); Expression spanTags2 = createComplexAttributeExpression("Span.tags", "span.kind").build(); @@ -202,6 +205,8 @@ void transQueryWithComplexAttributeExpression_HierarchicalFilter() { @Test void transQueryWithComplexAttributeExpression_OrderByAndFilter() { this.mockAttribute("server", AttributeMetadata.getDefaultInstance()); + this.mockAttribute("Span.tags", AttributeMetadata.getDefaultInstance()); + Expression.Builder spanTag = createComplexAttributeExpression("Span.tags", "span.kind"); Filter filter = @@ -237,6 +242,8 @@ void transQueryWithComplexAttributeExpression_OrderByAndFilter() { @Test void transQueryWithComplexAttributeExpression_SingleOrderBy() { + this.mockAttribute("Span.tags", AttributeMetadata.getDefaultInstance()); + Expression.Builder spanTag = createComplexAttributeExpression("Span.tags", "span.kind"); QueryRequest originalRequest = @@ -259,6 +266,8 @@ void transQueryWithComplexAttributeExpression_SingleOrderBy() { @Test void transQueryWithComplexAttributeExpression_MultipleOrderBy() { + this.mockAttribute("Span.tags", AttributeMetadata.getDefaultInstance()); + Expression.Builder spanTag1 = createComplexAttributeExpression("Span.tags", "span.kind"); Expression.Builder spanTag2 = createComplexAttributeExpression("Span.tags", "FLAGS"); From a1b9cfefb6002b7cbc30497a4909b5e438e7d7ab Mon Sep 17 00:00:00 2001 From: Sarthak Singhal Date: Thu, 23 Dec 2021 10:47:10 +0530 Subject: [PATCH 11/23] added template for new tests --- .../htqueries/AttributeExpressionQueries.java | 32 +++++++ .../query1.json | 90 +++++++++++++++++++ 2 files changed, 122 insertions(+) create mode 100644 query-service/src/integrationTest/java/org/hypertrace/core/query/service/htqueries/AttributeExpressionQueries.java create mode 100644 query-service/src/integrationTest/resources/attribute-expression-test-queries/query1.json diff --git a/query-service/src/integrationTest/java/org/hypertrace/core/query/service/htqueries/AttributeExpressionQueries.java b/query-service/src/integrationTest/java/org/hypertrace/core/query/service/htqueries/AttributeExpressionQueries.java new file mode 100644 index 00000000..0fd8e728 --- /dev/null +++ b/query-service/src/integrationTest/java/org/hypertrace/core/query/service/htqueries/AttributeExpressionQueries.java @@ -0,0 +1,32 @@ +package org.hypertrace.core.query.service.htqueries; + +import com.google.protobuf.InvalidProtocolBufferException; +import com.google.protobuf.util.JsonFormat; +import java.io.BufferedReader; +import java.io.IOException; +import java.io.InputStream; +import java.io.InputStreamReader; +import java.io.Reader; +import org.apache.kafka.common.requests.RequestContext; +import org.hypertrace.core.query.service.api.QueryRequest; + +class AttributeExpressionQueries { + + private static Reader readResourceFile(String fileName) { + InputStream in = RequestContext.class.getClassLoader().getResourceAsStream(fileName); + return new BufferedReader(new InputStreamReader(in)); + } + + // format for new tests + static QueryRequest buildQuery1() throws IOException { + String resourceFileName = "attribute-expression-test-queries/query1.json"; + Reader requestJsonStr = readResourceFile(resourceFileName); + QueryRequest.Builder builder = QueryRequest.newBuilder(); + try { + JsonFormat.parser().merge(requestJsonStr, builder); + } catch (InvalidProtocolBufferException e) { + e.printStackTrace(); + } + return builder.build(); + } +} diff --git a/query-service/src/integrationTest/resources/attribute-expression-test-queries/query1.json b/query-service/src/integrationTest/resources/attribute-expression-test-queries/query1.json new file mode 100644 index 00000000..bc37b24e --- /dev/null +++ b/query-service/src/integrationTest/resources/attribute-expression-test-queries/query1.json @@ -0,0 +1,90 @@ +{ + "filter": { + "childFilter": [{ + "lhs": { + "columnIdentifier": { + "columnName": "SERVICE.startTime" + } + }, + "operator": "GE", + "rhs": { + "literal": { + "value": { + "valueType": "LONG", + "long": "1640231115416" + } + } + } + }, { + "lhs": { + "columnIdentifier": { + "columnName": "SERVICE.startTime" + } + }, + "operator": "LT", + "rhs": { + "literal": { + "value": { + "valueType": "LONG", + "long": "1640234717266" + } + } + } + }, { + "lhs": { + "columnIdentifier": { + "columnName": "SERVICE.id" + } + }, + "operator": "NEQ", + "rhs": { + "literal": { + "value": { + "valueType": "NULL_STRING" + } + } + } + }] + }, + "selection": [{ + "columnIdentifier": { + "columnName": "SERVICE.id" + } + }, { + "columnIdentifier": { + "columnName": "SERVICE.name" + } + }, { + "function": { + "functionName": "COUNT", + "arguments": [{ + "columnIdentifier": { + "columnName": "SERVICE.id" + } + }] + } + }], + "groupBy": [{ + "columnIdentifier": { + "columnName": "SERVICE.id" + } + }, { + "columnIdentifier": { + "columnName": "SERVICE.name" + } + }], + "orderBy": [{ + "expression": { + "function": { + "functionName": "PERCENTILE99", + "arguments": [{ + "columnIdentifier": { + "columnName": "SERVICE.duration" + } + }] + } + }, + "order": "DESC" + }], + "limit": 10000 +} \ No newline at end of file From 46132ca5da26079b70fd5f28dc681d9dd25d17b4 Mon Sep 17 00:00:00 2001 From: Sarthak Singhal Date: Thu, 23 Dec 2021 10:59:21 +0530 Subject: [PATCH 12/23] added integ test with attr expr for existing queries --- query-service/build.gradle.kts | 1 + .../query/service/QueryServiceTestUtils.java | 25 ++++++-- .../service/htqueries/ExplorerQueries.java | 47 --------------- .../service/htqueries/HTPinotQueriesTest.java | 59 +++++++++++++------ 4 files changed, 61 insertions(+), 71 deletions(-) diff --git a/query-service/build.gradle.kts b/query-service/build.gradle.kts index 51d72641..bbacfd45 100644 --- a/query-service/build.gradle.kts +++ b/query-service/build.gradle.kts @@ -13,6 +13,7 @@ dependencies { implementation("org.hypertrace.core.grpcutils:grpc-server-utils:0.6.2") implementation("org.hypertrace.core.serviceframework:platform-service-framework:0.1.28") implementation("org.slf4j:slf4j-api:1.7.30") + implementation("com.google.protobuf:protobuf-java-util:3.17.3") implementation("com.typesafe:config:1.4.1") runtimeOnly("org.apache.logging.log4j:log4j-slf4j-impl:2.17.0") diff --git a/query-service/src/integrationTest/java/org/hypertrace/core/query/service/QueryServiceTestUtils.java b/query-service/src/integrationTest/java/org/hypertrace/core/query/service/QueryServiceTestUtils.java index eb69b8f9..945b7b65 100644 --- a/query-service/src/integrationTest/java/org/hypertrace/core/query/service/QueryServiceTestUtils.java +++ b/query-service/src/integrationTest/java/org/hypertrace/core/query/service/QueryServiceTestUtils.java @@ -1,6 +1,7 @@ package org.hypertrace.core.query.service; -import org.hypertrace.core.query.service.api.AttributeExpression; +import com.google.protobuf.InvalidProtocolBufferException; +import com.google.protobuf.util.JsonFormat; import org.hypertrace.core.query.service.api.ColumnIdentifier; import org.hypertrace.core.query.service.api.Expression; import org.hypertrace.core.query.service.api.Filter; @@ -8,6 +9,7 @@ import org.hypertrace.core.query.service.api.LiteralConstant; import org.hypertrace.core.query.service.api.Operator; import org.hypertrace.core.query.service.api.OrderByExpression; +import org.hypertrace.core.query.service.api.QueryRequest; import org.hypertrace.core.query.service.api.SortOrder; import org.hypertrace.core.query.service.api.Value; import org.hypertrace.core.query.service.api.ValueType; @@ -103,10 +105,21 @@ public static Expression createStringLiteralValueExpression(String value) { .build(); } - public static Expression.Builder createComplexAttributeExpression( - String attributeId, String subPath) { - return Expression.newBuilder() - .setAttributeExpression( - AttributeExpression.newBuilder().setAttributeId(attributeId).setSubpath(subPath)); + public static QueryRequest getAttributeExpressionQuery(QueryRequest originalQueryRequest) + throws InvalidProtocolBufferException { + // Serialize into json. + String json = + JsonFormat.printer().omittingInsignificantWhitespace().print(originalQueryRequest); + System.out.println(json); + + // Change for attribute Expression + json = json.replaceAll("columnIdentifier", "attributeExpression"); + json = json.replaceAll("columnName", "attributeId"); + System.out.println(json); + + // Deserialize and return + QueryRequest.Builder newBuilder = QueryRequest.newBuilder(); + JsonFormat.parser().merge(json, newBuilder); + return newBuilder.build(); } } diff --git a/query-service/src/integrationTest/java/org/hypertrace/core/query/service/htqueries/ExplorerQueries.java b/query-service/src/integrationTest/java/org/hypertrace/core/query/service/htqueries/ExplorerQueries.java index 21b31688..5a00dee1 100644 --- a/query-service/src/integrationTest/java/org/hypertrace/core/query/service/htqueries/ExplorerQueries.java +++ b/query-service/src/integrationTest/java/org/hypertrace/core/query/service/htqueries/ExplorerQueries.java @@ -1,10 +1,6 @@ package org.hypertrace.core.query.service.htqueries; -import static org.hypertrace.core.query.service.QueryRequestUtil.createContainsKeyFilter; -import static org.hypertrace.core.query.service.QueryServiceTestUtils.createComplexAttributeExpression; import static org.hypertrace.core.query.service.QueryServiceTestUtils.createFilter; -import static org.hypertrace.core.query.service.QueryServiceTestUtils.createOrderByExpression; -import static org.hypertrace.core.query.service.QueryServiceTestUtils.createStringLiteralValueExpression; import java.time.Duration; import org.hypertrace.core.query.service.api.ColumnIdentifier; @@ -15,7 +11,6 @@ import org.hypertrace.core.query.service.api.Operator; import org.hypertrace.core.query.service.api.QueryRequest; import org.hypertrace.core.query.service.api.QueryRequest.Builder; -import org.hypertrace.core.query.service.api.SortOrder; import org.hypertrace.core.query.service.api.Value; import org.hypertrace.core.query.service.api.ValueType; @@ -98,46 +93,4 @@ static QueryRequest buildQuery1() { builder.addGroupBy(Expression.newBuilder().setFunction(dateTimeConvert).build()); return builder.build(); } - - static QueryRequest buildQuery2() { - Builder builder = QueryRequest.newBuilder(); - - Expression apiTraceTags = - createComplexAttributeExpression("API_TRACE.tags", "span.kind").build(); - builder.addSelection(apiTraceTags); - - Filter equalFilter = - Filter.newBuilder() - .setOperator(Operator.EQ) - .setLhs(apiTraceTags) - .setRhs(createStringLiteralValueExpression("client")) - .build(); - - Filter startTimeFilter = - createFilter( - "API_TRACE.startTime", - Operator.GE, - ValueType.LONG, - System.currentTimeMillis() - Duration.ofHours(1).toMillis()); - - Filter endTimeFilter = - createFilter( - "API_TRACE.startTime", - Operator.LT, - ValueType.LONG, - System.currentTimeMillis() + Duration.ofHours(1).toMillis()); - - builder.setFilter( - Filter.newBuilder() - .setOperator(Operator.AND) - .addChildFilter(startTimeFilter) - .addChildFilter(endTimeFilter) - .addChildFilter(equalFilter) - .addChildFilter(createContainsKeyFilter("API_TRACE.tags", "span.kind")) - .build()); - - builder.addOrderBy(createOrderByExpression(apiTraceTags, SortOrder.DESC)); - - return builder.build(); - } } diff --git a/query-service/src/integrationTest/java/org/hypertrace/core/query/service/htqueries/HTPinotQueriesTest.java b/query-service/src/integrationTest/java/org/hypertrace/core/query/service/htqueries/HTPinotQueriesTest.java index f38b3f70..697eed63 100644 --- a/query-service/src/integrationTest/java/org/hypertrace/core/query/service/htqueries/HTPinotQueriesTest.java +++ b/query-service/src/integrationTest/java/org/hypertrace/core/query/service/htqueries/HTPinotQueriesTest.java @@ -1,12 +1,14 @@ package org.hypertrace.core.query.service.htqueries; import static com.github.stefanbirkner.systemlambda.SystemLambda.withEnvironmentVariable; +import static org.hypertrace.core.query.service.QueryServiceTestUtils.getAttributeExpressionQuery; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertTrue; import com.google.common.collect.ImmutableMap; import com.google.common.collect.Maps; import com.google.common.collect.Streams; +import com.google.protobuf.InvalidProtocolBufferException; import com.typesafe.config.ConfigFactory; import io.grpc.ManagedChannel; import io.grpc.ManagedChannelBuilder; @@ -19,6 +21,7 @@ import java.util.Map; import java.util.UUID; import java.util.stream.Collectors; +import java.util.stream.Stream; import org.apache.avro.file.DataFileReader; import org.apache.avro.specific.SpecificDatumReader; import org.apache.kafka.clients.admin.AdminClient; @@ -34,6 +37,7 @@ import org.hypertrace.core.attribute.service.v1.AttributeMetadataFilter; import org.hypertrace.core.datamodel.StructuredTrace; import org.hypertrace.core.kafkastreams.framework.serdes.AvroSerde; +import org.hypertrace.core.query.service.api.QueryRequest; import org.hypertrace.core.query.service.api.ResultSetChunk; import org.hypertrace.core.query.service.api.Row; import org.hypertrace.core.query.service.client.QueryServiceClient; @@ -42,6 +46,9 @@ import org.junit.jupiter.api.AfterAll; import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.Arguments; +import org.junit.jupiter.params.provider.MethodSource; import org.slf4j.Logger; import org.slf4j.LoggerFactory; import org.testcontainers.containers.GenericContainer; @@ -313,11 +320,12 @@ private static void validateRows(List rows, double divisor) { }); } - @Test - public void testServicesQueries() { + @ParameterizedTest + @MethodSource("provideQueryRequestForServiceQueries") + public void testServicesQueries(QueryRequest queryRequest) { LOG.info("Services queries"); Iterator itr = - queryServiceClient.executeQuery(ServicesQueries.buildQuery1(), TENANT_ID_MAP, 10000); + queryServiceClient.executeQuery(queryRequest, TENANT_ID_MAP, 10000); List list = Streams.stream(itr).collect(Collectors.toList()); List rows = list.get(0).getRowList(); assertEquals(4, rows.size()); @@ -369,11 +377,12 @@ public void testServicesQueriesForAvgRateWithTimeAggregation() { validateRows(rows, 15); } - @Test - public void testBackendsQueries() { + @ParameterizedTest + @MethodSource("provideQueryRequestForBackendQueries") + public void testBackendsQueries(QueryRequest queryRequest) { LOG.info("Backends queries"); Iterator itr = - queryServiceClient.executeQuery(BackendsQueries.buildQuery1(), TENANT_ID_MAP, 10000); + queryServiceClient.executeQuery(queryRequest, TENANT_ID_MAP, 10000); List list = Streams.stream(itr).collect(Collectors.toList()); List rows = list.get(0).getRowList(); assertEquals(1, rows.size()); @@ -382,11 +391,12 @@ public void testBackendsQueries() { assertTrue(backendNames.isEmpty()); } - @Test - public void testExplorerQueries() { + @ParameterizedTest + @MethodSource("provideQueryRequestForExplorerQueries") + public void testExplorerQueries(QueryRequest queryRequest) { LOG.info("Explorer queries"); Iterator itr = - queryServiceClient.executeQuery(ExplorerQueries.buildQuery1(), TENANT_ID_MAP, 10000); + queryServiceClient.executeQuery(queryRequest, TENANT_ID_MAP, 10000); List list = Streams.stream(itr).collect(Collectors.toList()); List rows = list.get(0).getRowList(); assertEquals(1, rows.size()); @@ -394,14 +404,27 @@ public void testExplorerQueries() { assertEquals("13", rows.get(0).getColumn(1).getString()); } - @Test - public void testExplorerQueriesForAttributeExpression() { - LOG.info("Explorer queries for attribute expression"); - Iterator itr = - queryServiceClient.executeQuery(ExplorerQueries.buildQuery2(), TENANT_ID_MAP, 10000); - List list = Streams.stream(itr).collect(Collectors.toList()); - List rows = list.get(0).getRowList(); - assertEquals(10, rows.size()); - assertEquals("client", rows.get(0).getColumn(0).getString()); + private static Stream provideQueryRequestForServiceQueries() + throws InvalidProtocolBufferException { + QueryRequest queryRequest1 = ServicesQueries.buildQuery1(); + return Stream.of( + Arguments.arguments(queryRequest1), + Arguments.arguments(getAttributeExpressionQuery(queryRequest1))); + } + + private static Stream provideQueryRequestForBackendQueries() + throws InvalidProtocolBufferException { + QueryRequest queryRequest1 = BackendsQueries.buildQuery1(); + return Stream.of( + Arguments.arguments(queryRequest1), + Arguments.arguments(getAttributeExpressionQuery(queryRequest1))); + } + + private static Stream provideQueryRequestForExplorerQueries() + throws InvalidProtocolBufferException { + QueryRequest queryRequest1 = ExplorerQueries.buildQuery1(); + return Stream.of( + Arguments.arguments(queryRequest1), + Arguments.arguments(getAttributeExpressionQuery(queryRequest1))); } } From 75aad05e23a9e3ebbcc63ab62bf8b52469d93cff Mon Sep 17 00:00:00 2001 From: Sarthak Singhal Date: Thu, 23 Dec 2021 12:32:51 +0530 Subject: [PATCH 13/23] added functions for integ test --- .../query/service/QueryServiceTestUtils.java | 25 ++++++ .../htqueries/AttributeExpressionQueries.java | 32 ------- .../query1.json | 73 ++++------------ .../query2.json | 85 +++++++++++++++++++ 4 files changed, 126 insertions(+), 89 deletions(-) delete mode 100644 query-service/src/integrationTest/java/org/hypertrace/core/query/service/htqueries/AttributeExpressionQueries.java create mode 100644 query-service/src/integrationTest/resources/attribute-expression-test-queries/query2.json diff --git a/query-service/src/integrationTest/java/org/hypertrace/core/query/service/QueryServiceTestUtils.java b/query-service/src/integrationTest/java/org/hypertrace/core/query/service/QueryServiceTestUtils.java index 945b7b65..66cb6a61 100644 --- a/query-service/src/integrationTest/java/org/hypertrace/core/query/service/QueryServiceTestUtils.java +++ b/query-service/src/integrationTest/java/org/hypertrace/core/query/service/QueryServiceTestUtils.java @@ -2,6 +2,12 @@ import com.google.protobuf.InvalidProtocolBufferException; import com.google.protobuf.util.JsonFormat; +import java.io.BufferedReader; +import java.io.IOException; +import java.io.InputStream; +import java.io.InputStreamReader; +import java.io.Reader; +import org.apache.kafka.common.requests.RequestContext; import org.hypertrace.core.query.service.api.ColumnIdentifier; import org.hypertrace.core.query.service.api.Expression; import org.hypertrace.core.query.service.api.Filter; @@ -16,6 +22,8 @@ public class QueryServiceTestUtils { + private static final String REQUESTS_DIR = "attribute-expression-test-queries"; + public static Filter createFilter( String columnName, Operator op, ValueType valueType, Object valueObject) { ColumnIdentifier startTimeColumn = @@ -122,4 +130,21 @@ public static QueryRequest getAttributeExpressionQuery(QueryRequest originalQuer JsonFormat.parser().merge(json, newBuilder); return newBuilder.build(); } + + public static QueryRequest buildQueryFromJsonFile(String filename) throws IOException { + String resourceFileName = REQUESTS_DIR + "/" + filename; + Reader requestJsonStr = readResourceFile(resourceFileName); + QueryRequest.Builder builder = QueryRequest.newBuilder(); + try { + JsonFormat.parser().merge(requestJsonStr, builder); + } catch (InvalidProtocolBufferException e) { + e.printStackTrace(); + } + return builder.build(); + } + + private static Reader readResourceFile(String fileName) { + InputStream in = RequestContext.class.getClassLoader().getResourceAsStream(fileName); + return new BufferedReader(new InputStreamReader(in)); + } } diff --git a/query-service/src/integrationTest/java/org/hypertrace/core/query/service/htqueries/AttributeExpressionQueries.java b/query-service/src/integrationTest/java/org/hypertrace/core/query/service/htqueries/AttributeExpressionQueries.java deleted file mode 100644 index 0fd8e728..00000000 --- a/query-service/src/integrationTest/java/org/hypertrace/core/query/service/htqueries/AttributeExpressionQueries.java +++ /dev/null @@ -1,32 +0,0 @@ -package org.hypertrace.core.query.service.htqueries; - -import com.google.protobuf.InvalidProtocolBufferException; -import com.google.protobuf.util.JsonFormat; -import java.io.BufferedReader; -import java.io.IOException; -import java.io.InputStream; -import java.io.InputStreamReader; -import java.io.Reader; -import org.apache.kafka.common.requests.RequestContext; -import org.hypertrace.core.query.service.api.QueryRequest; - -class AttributeExpressionQueries { - - private static Reader readResourceFile(String fileName) { - InputStream in = RequestContext.class.getClassLoader().getResourceAsStream(fileName); - return new BufferedReader(new InputStreamReader(in)); - } - - // format for new tests - static QueryRequest buildQuery1() throws IOException { - String resourceFileName = "attribute-expression-test-queries/query1.json"; - Reader requestJsonStr = readResourceFile(resourceFileName); - QueryRequest.Builder builder = QueryRequest.newBuilder(); - try { - JsonFormat.parser().merge(requestJsonStr, builder); - } catch (InvalidProtocolBufferException e) { - e.printStackTrace(); - } - return builder.build(); - } -} diff --git a/query-service/src/integrationTest/resources/attribute-expression-test-queries/query1.json b/query-service/src/integrationTest/resources/attribute-expression-test-queries/query1.json index bc37b24e..c1e854db 100644 --- a/query-service/src/integrationTest/resources/attribute-expression-test-queries/query1.json +++ b/query-service/src/integrationTest/resources/attribute-expression-test-queries/query1.json @@ -2,89 +2,48 @@ "filter": { "childFilter": [{ "lhs": { - "columnIdentifier": { - "columnName": "SERVICE.startTime" + "attributeExpression": { + "attributeId": "Span.tags" } }, - "operator": "GE", - "rhs": { - "literal": { - "value": { - "valueType": "LONG", - "long": "1640231115416" - } - } - } - }, { - "lhs": { - "columnIdentifier": { - "columnName": "SERVICE.startTime" - } - }, - "operator": "LT", + "operator": "CONTAINS_KEY", "rhs": { "literal": { "value": { - "valueType": "LONG", - "long": "1640234717266" + "string": "span.kind" } } } }, { "lhs": { - "columnIdentifier": { - "columnName": "SERVICE.id" + "attributeExpression": { + "attributeId": "Span.tags", + "subpath": "span.kind" } }, - "operator": "NEQ", + "operator": "GE", "rhs": { "literal": { "value": { - "valueType": "NULL_STRING" + "string": "client" } } } }] }, "selection": [{ - "columnIdentifier": { - "columnName": "SERVICE.id" - } - }, { - "columnIdentifier": { - "columnName": "SERVICE.name" - } - }, { - "function": { - "functionName": "COUNT", - "arguments": [{ - "columnIdentifier": { - "columnName": "SERVICE.id" - } - }] - } - }], - "groupBy": [{ - "columnIdentifier": { - "columnName": "SERVICE.id" - } - }, { - "columnIdentifier": { - "columnName": "SERVICE.name" + "attributeExpression": { + "attributeId": "Span.tags", + "subpath": "span.kind" } }], "orderBy": [{ "expression": { - "function": { - "functionName": "PERCENTILE99", - "arguments": [{ - "columnIdentifier": { - "columnName": "SERVICE.duration" - } - }] + "attributeExpression": { + "attributeId": "Span.tags", + "subpath": "span.kind" } }, "order": "DESC" - }], - "limit": 10000 + }] } \ No newline at end of file diff --git a/query-service/src/integrationTest/resources/attribute-expression-test-queries/query2.json b/query-service/src/integrationTest/resources/attribute-expression-test-queries/query2.json new file mode 100644 index 00000000..495bb845 --- /dev/null +++ b/query-service/src/integrationTest/resources/attribute-expression-test-queries/query2.json @@ -0,0 +1,85 @@ +{ + "filter": { + "childFilter": [{ + "lhs": { + "attributeExpression": { + "attributeId": "Span.start_time_millis" + } + }, + "operator": "GT", + "rhs": { + "literal": { + "value": { + "valueType": "LONG", + "long": "1570658506605" + } + } + } + }, { + "lhs": { + "attributeExpression": { + "attributeId": "Span.start_time_millis" + } + }, + "operator": "LT", + "rhs": { + "literal": { + "value": { + "valueType": "LONG", + "long": "1570744906673" + } + } + } + }, { + "lhs": { + "attributeExpression": { + "attributeId": "Span.tags" + } + }, + "operator": "CONTAINS_KEY", + "rhs": { + "literal": { + "value": { + "string": "span.kind" + } + } + } + }, { + "lhs": { + "attributeExpression": { + "attributeId": "Span.tags", + "subpath": "span.kind" + } + }, + "operator": "NEQ", + "rhs": { + "literal": { + "value": { + } + } + } + }] + }, + "selection": [{ + "function": { + "functionName": "AVG", + "arguments": [{ + "attributeExpression": { + "attributeId": "Span.duration_millis" + } + }], + "alias": "avg_duration" + } + }, { + "attributeExpression": { + "attributeId": "Span.tags", + "subpath": "span.kind" + } + }], + "groupBy": [{ + "attributeExpression": { + "attributeId": "Span.tags", + "subpath": "span.kind" + } + }] +} \ No newline at end of file From 0a57dd95bd658916030378223600e3d41c860c62 Mon Sep 17 00:00:00 2001 From: Sarthak Singhal Date: Thu, 23 Dec 2021 13:22:45 +0530 Subject: [PATCH 14/23] added new integ tests --- .../service/htqueries/HTPinotQueriesTest.java | 22 +++++++++++++++++++ .../query1.json | 8 +++---- .../query2.json | 16 +++++++------- 3 files changed, 34 insertions(+), 12 deletions(-) diff --git a/query-service/src/integrationTest/java/org/hypertrace/core/query/service/htqueries/HTPinotQueriesTest.java b/query-service/src/integrationTest/java/org/hypertrace/core/query/service/htqueries/HTPinotQueriesTest.java index 697eed63..957aa6d8 100644 --- a/query-service/src/integrationTest/java/org/hypertrace/core/query/service/htqueries/HTPinotQueriesTest.java +++ b/query-service/src/integrationTest/java/org/hypertrace/core/query/service/htqueries/HTPinotQueriesTest.java @@ -1,6 +1,7 @@ package org.hypertrace.core.query.service.htqueries; import static com.github.stefanbirkner.systemlambda.SystemLambda.withEnvironmentVariable; +import static org.hypertrace.core.query.service.QueryServiceTestUtils.buildQueryFromJsonFile; import static org.hypertrace.core.query.service.QueryServiceTestUtils.getAttributeExpressionQuery; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertTrue; @@ -13,6 +14,7 @@ import io.grpc.ManagedChannel; import io.grpc.ManagedChannelBuilder; import java.io.File; +import java.io.IOException; import java.util.ArrayList; import java.util.Arrays; import java.util.Collections; @@ -404,6 +406,19 @@ public void testExplorerQueries(QueryRequest queryRequest) { assertEquals("13", rows.get(0).getColumn(1).getString()); } + @ParameterizedTest + @MethodSource("provideQueryRequestForAttributeExpressionQueries") + public void testAttributeExpressionQueries( + QueryRequest queryRequest, int rowSize, String expectedValue) { + LOG.info("Attribute Expression queries"); + Iterator itr = + queryServiceClient.executeQuery(queryRequest, TENANT_ID_MAP, 10000); + List list = Streams.stream(itr).collect(Collectors.toList()); + List rows = list.get(0).getRowList(); + assertEquals(rowSize, rows.size()); + assertEquals(expectedValue, rows.get(0).getColumn(0).getString()); + } + private static Stream provideQueryRequestForServiceQueries() throws InvalidProtocolBufferException { QueryRequest queryRequest1 = ServicesQueries.buildQuery1(); @@ -427,4 +442,11 @@ private static Stream provideQueryRequestForExplorerQueries() Arguments.arguments(queryRequest1), Arguments.arguments(getAttributeExpressionQuery(queryRequest1))); } + + private static Stream provideQueryRequestForAttributeExpressionQueries() + throws IOException { + return Stream.of( + Arguments.arguments(buildQueryFromJsonFile("query1.json"), 10, "server"), + Arguments.arguments(buildQueryFromJsonFile("query2.json"), 2, "server")); + } } diff --git a/query-service/src/integrationTest/resources/attribute-expression-test-queries/query1.json b/query-service/src/integrationTest/resources/attribute-expression-test-queries/query1.json index c1e854db..5134a4e1 100644 --- a/query-service/src/integrationTest/resources/attribute-expression-test-queries/query1.json +++ b/query-service/src/integrationTest/resources/attribute-expression-test-queries/query1.json @@ -3,7 +3,7 @@ "childFilter": [{ "lhs": { "attributeExpression": { - "attributeId": "Span.tags" + "attributeId": "EVENT.spanTags" } }, "operator": "CONTAINS_KEY", @@ -17,7 +17,7 @@ }, { "lhs": { "attributeExpression": { - "attributeId": "Span.tags", + "attributeId": "EVENT.spanTags", "subpath": "span.kind" } }, @@ -33,14 +33,14 @@ }, "selection": [{ "attributeExpression": { - "attributeId": "Span.tags", + "attributeId": "EVENT.spanTags", "subpath": "span.kind" } }], "orderBy": [{ "expression": { "attributeExpression": { - "attributeId": "Span.tags", + "attributeId": "EVENT.spanTags", "subpath": "span.kind" } }, diff --git a/query-service/src/integrationTest/resources/attribute-expression-test-queries/query2.json b/query-service/src/integrationTest/resources/attribute-expression-test-queries/query2.json index 495bb845..5b746cf9 100644 --- a/query-service/src/integrationTest/resources/attribute-expression-test-queries/query2.json +++ b/query-service/src/integrationTest/resources/attribute-expression-test-queries/query2.json @@ -3,7 +3,7 @@ "childFilter": [{ "lhs": { "attributeExpression": { - "attributeId": "Span.start_time_millis" + "attributeId": "EVENT.startTime" } }, "operator": "GT", @@ -18,7 +18,7 @@ }, { "lhs": { "attributeExpression": { - "attributeId": "Span.start_time_millis" + "attributeId": "EVENT.startTime" } }, "operator": "LT", @@ -26,14 +26,14 @@ "literal": { "value": { "valueType": "LONG", - "long": "1570744906673" + "long": "2570744906673" } } } }, { "lhs": { "attributeExpression": { - "attributeId": "Span.tags" + "attributeId": "EVENT.spanTags" } }, "operator": "CONTAINS_KEY", @@ -47,7 +47,7 @@ }, { "lhs": { "attributeExpression": { - "attributeId": "Span.tags", + "attributeId": "EVENT.spanTags", "subpath": "span.kind" } }, @@ -65,20 +65,20 @@ "functionName": "AVG", "arguments": [{ "attributeExpression": { - "attributeId": "Span.duration_millis" + "attributeId": "EVENT.duration" } }], "alias": "avg_duration" } }, { "attributeExpression": { - "attributeId": "Span.tags", + "attributeId": "EVENT.spanTags", "subpath": "span.kind" } }], "groupBy": [{ "attributeExpression": { - "attributeId": "Span.tags", + "attributeId": "EVENT.spanTags", "subpath": "span.kind" } }] From 929e3c9d3ca1761238bfa7e8ddbe4c25d7c27fc7 Mon Sep 17 00:00:00 2001 From: Sarthak Singhal Date: Thu, 23 Dec 2021 21:09:42 +0530 Subject: [PATCH 15/23] nit --- query-service/build.gradle.kts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/query-service/build.gradle.kts b/query-service/build.gradle.kts index bbacfd45..edec2e29 100644 --- a/query-service/build.gradle.kts +++ b/query-service/build.gradle.kts @@ -13,7 +13,6 @@ dependencies { implementation("org.hypertrace.core.grpcutils:grpc-server-utils:0.6.2") implementation("org.hypertrace.core.serviceframework:platform-service-framework:0.1.28") implementation("org.slf4j:slf4j-api:1.7.30") - implementation("com.google.protobuf:protobuf-java-util:3.17.3") implementation("com.typesafe:config:1.4.1") runtimeOnly("org.apache.logging.log4j:log4j-slf4j-impl:2.17.0") @@ -27,6 +26,7 @@ dependencies { } } + integrationTestImplementation("com.google.protobuf:protobuf-java-util:3.17.3") integrationTestImplementation("org.junit.jupiter:junit-jupiter-api:5.7.1") integrationTestImplementation("org.junit.jupiter:junit-jupiter-params:5.7.1") integrationTestImplementation("org.junit.jupiter:junit-jupiter-engine:5.7.1") From d51263a40a2124ce0cd2ba082447be5b3446bb71 Mon Sep 17 00:00:00 2001 From: Sarthak Singhal Date: Thu, 23 Dec 2021 21:59:14 +0530 Subject: [PATCH 16/23] added contains key filter for selection --- .../projection/ProjectionTransformation.java | 39 +++++++++++++++---- .../ProjectionTransformationTest.java | 21 ++++++++++ 2 files changed, 52 insertions(+), 8 deletions(-) diff --git a/query-service-impl/src/main/java/org/hypertrace/core/query/service/projection/ProjectionTransformation.java b/query-service-impl/src/main/java/org/hypertrace/core/query/service/projection/ProjectionTransformation.java index 3216bb61..f1e9b819 100644 --- a/query-service-impl/src/main/java/org/hypertrace/core/query/service/projection/ProjectionTransformation.java +++ b/query-service-impl/src/main/java/org/hypertrace/core/query/service/projection/ProjectionTransformation.java @@ -17,6 +17,7 @@ import java.util.List; import java.util.Optional; import java.util.stream.Collectors; +import java.util.stream.Stream; import javax.inject.Inject; import org.hypertrace.core.attribute.service.cachingclient.CachingAttributeClient; import org.hypertrace.core.attribute.service.projection.AttributeProjection; @@ -344,7 +345,8 @@ private QueryRequest rebuildRequest( List orderBys) { QueryRequest.Builder builder = original.toBuilder(); - Filter updatedFilter = rebuildFilterForComplexAttributeExpression(originalFilter, orderBys); + Filter updatedFilter = + rebuildFilterForComplexAttributeExpression(originalFilter, orderBys, selections); if (Filter.getDefaultInstance().equals(updatedFilter)) { builder.clearFilter(); @@ -365,16 +367,20 @@ private QueryRequest rebuildRequest( } /* - * We need the CONTAINS_KEY filter in all filters and order bys dealing with complex + * We need the CONTAINS_KEY filter in all filters, selections and order bys dealing with complex * attribute expressions as Pinot gives error if particular key is absent. Rest all work fine. - * To handle order bys, we add the corresponding filter at the top and 'AND' it with the main filter. + * To handle order bys and selections, we add the corresponding filter at the top and 'AND' it with the main filter. * To handle filter, we modify each filter (say filter1) as : "CONTAINS_KEY AND filter1". */ private Filter rebuildFilterForComplexAttributeExpression( - Filter originalFilter, List orderBys) { + Filter originalFilter, List orderBys, List selections) { Filter updatedFilter = updateFilterForComplexAttributeExpressionFromFilter(originalFilter); - List filterList = createFilterForComplexAttributeExpressionFromOrderBy(orderBys); + List filterList = + Stream.concat( + createFilterForComplexAttributeExpressionFromOrderBy(orderBys), + createFilterForComplexAttributeExpressionFromSelection(selections)) + .collect(Collectors.toList()); if (filterList.isEmpty()) { return updatedFilter; @@ -423,14 +429,31 @@ private Filter updateFilterForComplexAttributeExpressionFromFilter(Filter origin } } - private List createFilterForComplexAttributeExpressionFromOrderBy( + private Stream createFilterForComplexAttributeExpressionFromOrderBy( List orderByExpressionList) { return orderByExpressionList.stream() .map(OrderByExpression::getExpression) .filter(QueryRequestUtil::isAttributeExpressionWithSubpath) .map(Expression::getAttributeExpression) - .map(QueryRequestUtil::createContainsKeyFilter) - .collect(Collectors.toList()); + .map(QueryRequestUtil::createContainsKeyFilter); + } + + private Stream createFilterForComplexAttributeExpressionFromSelection( + List selections) { + return selections.stream() + .flatMap(this::getAnyAttributeExpression) + .map(QueryRequestUtil::createContainsKeyFilter); + } + + private Stream getAnyAttributeExpression(Expression selection) { + if (selection.hasFunction()) { + return selection.getFunction().getArgumentsList().stream() + .flatMap(this::getAnyAttributeExpression); + } else { + return Stream.of(selection) + .filter(QueryRequestUtil::isAttributeExpressionWithSubpath) + .map(Expression::getAttributeExpression); + } } private String getOriginalKey(AttributeExpression attributeExpression) { diff --git a/query-service-impl/src/test/java/org/hypertrace/core/query/service/projection/ProjectionTransformationTest.java b/query-service-impl/src/test/java/org/hypertrace/core/query/service/projection/ProjectionTransformationTest.java index a1654f3f..4ded70c5 100644 --- a/query-service-impl/src/test/java/org/hypertrace/core/query/service/projection/ProjectionTransformationTest.java +++ b/query-service-impl/src/test/java/org/hypertrace/core/query/service/projection/ProjectionTransformationTest.java @@ -240,6 +240,27 @@ void transQueryWithComplexAttributeExpression_OrderByAndFilter() { .blockingGet()); } + @Test + void transQueryWithComplexAttributeExpression_SingleSelection() { + this.mockAttribute("Span.tags", AttributeMetadata.getDefaultInstance()); + + Expression.Builder spanTag = createComplexAttributeExpression("Span.tags", "span.kind"); + + QueryRequest originalRequest = QueryRequest.newBuilder().addSelection(spanTag).build(); + + QueryRequest expectedTransform = + QueryRequest.newBuilder() + .addSelection(spanTag) + .setFilter(createContainsKeyFilter("Span.tags", "span.kind")) + .build(); + + assertEquals( + expectedTransform, + this.projectionTransformation + .transform(originalRequest, mockTransformationContext) + .blockingGet()); + } + @Test void transQueryWithComplexAttributeExpression_SingleOrderBy() { this.mockAttribute("Span.tags", AttributeMetadata.getDefaultInstance()); From b4878a50aea98a8387722ee040931c5c3f19191d Mon Sep 17 00:00:00 2001 From: Aaron Steinfeld Date: Thu, 23 Dec 2021 11:53:22 -0500 Subject: [PATCH 17/23] fix: support for attribute expressioons in selections --- .../service/AbstractQueryTransformation.java | 167 ++++++++++ .../core/query/service/QueryRequestUtil.java | 20 +- .../query/service/QueryServiceModule.java | 2 + .../query/service/QueryTransformation.java | 12 +- .../service/QueryTransformationPipeline.java | 1 + .../AttributeNormalizationTransformation.java | 41 +++ ...eSubpathExistsFilteringTransformation.java | 133 ++++++++ .../ExpressionModule.java | 16 + .../projection/ProjectionTransformation.java | 292 ++---------------- 9 files changed, 405 insertions(+), 279 deletions(-) create mode 100644 query-service-impl/src/main/java/org/hypertrace/core/query/service/AbstractQueryTransformation.java create mode 100644 query-service-impl/src/main/java/org/hypertrace/core/query/service/attribubteexpression/AttributeNormalizationTransformation.java create mode 100644 query-service-impl/src/main/java/org/hypertrace/core/query/service/attribubteexpression/AttributeSubpathExistsFilteringTransformation.java create mode 100644 query-service-impl/src/main/java/org/hypertrace/core/query/service/attribubteexpression/ExpressionModule.java diff --git a/query-service-impl/src/main/java/org/hypertrace/core/query/service/AbstractQueryTransformation.java b/query-service-impl/src/main/java/org/hypertrace/core/query/service/AbstractQueryTransformation.java new file mode 100644 index 00000000..73434198 --- /dev/null +++ b/query-service-impl/src/main/java/org/hypertrace/core/query/service/AbstractQueryTransformation.java @@ -0,0 +1,167 @@ +package org.hypertrace.core.query.service; + +import static io.reactivex.rxjava3.core.Single.zip; + +import io.reactivex.rxjava3.core.Observable; +import io.reactivex.rxjava3.core.Single; +import java.util.List; +import org.hypertrace.core.query.service.api.AttributeExpression; +import org.hypertrace.core.query.service.api.ColumnIdentifier; +import org.hypertrace.core.query.service.api.Expression; +import org.hypertrace.core.query.service.api.Filter; +import org.hypertrace.core.query.service.api.Function; +import org.hypertrace.core.query.service.api.LiteralConstant; +import org.hypertrace.core.query.service.api.OrderByExpression; +import org.hypertrace.core.query.service.api.QueryRequest; +import org.slf4j.Logger; + +public abstract class AbstractQueryTransformation implements QueryTransformation { + + protected abstract Logger getLogger(); + + @Override + public Single transform( + QueryRequest queryRequest, QueryTransformationContext transformationContext) { + return zip( + this.transformExpressionList(queryRequest.getSelectionList()), + this.transformExpressionList(queryRequest.getAggregationList()), + this.transformFilter(queryRequest.getFilter()), + this.transformExpressionList(queryRequest.getGroupByList()), + this.transformOrderByList(queryRequest.getOrderByList()), + (selections, aggregations, filter, groupBys, orderBys) -> + this.rebuildRequest( + queryRequest, selections, aggregations, filter, groupBys, orderBys)) + .doOnSuccess(transformed -> this.debugLogIfRequestTransformed(queryRequest, transformed)); + } + + protected QueryRequest rebuildRequest( + QueryRequest original, + List selections, + List aggregations, + Filter filter, + List groupBys, + List orderBys) { + + return original.toBuilder() + .clearSelection() + .addAllSelection(selections) + .clearAggregation() + .addAllAggregation(aggregations) + .clearGroupBy() + .addAllGroupBy(groupBys) + .clearOrderBy() + .addAllOrderBy(orderBys) + .setFilter(filter) + .build(); + } + + protected Single transformExpression(Expression expression) { + switch (expression.getValueCase()) { + case COLUMNIDENTIFIER: + return this.transformColumnIdentifier(expression.getColumnIdentifier()); + case ATTRIBUTE_EXPRESSION: + return this.transformAttributeExpression(expression.getAttributeExpression()); + case FUNCTION: + return this.transformFunction(expression.getFunction()); + case ORDERBY: + return this.transformOrderBy(expression.getOrderBy()) + .map(expression.toBuilder()::setOrderBy) + .map(Expression.Builder::build); + case LITERAL: + return this.transformLiteral(expression.getLiteral()); + case VALUE_NOT_SET: + default: + return Single.just(expression); + } + } + + protected Single transformColumnIdentifier(ColumnIdentifier columnIdentifier) { + return Single.just(Expression.newBuilder().setColumnIdentifier(columnIdentifier).build()); + } + + protected Single transformAttributeExpression( + AttributeExpression attributeExpression) { + return Single.just(Expression.newBuilder().setAttributeExpression(attributeExpression).build()); + } + + protected Single transformLiteral(LiteralConstant literalConstant) { + return Single.just(Expression.newBuilder().setLiteral(literalConstant).build()); + } + + protected Single transformFunction(Function function) { + return this.transformExpressionList(function.getArgumentsList()) + .map(expressions -> function.toBuilder().clearArguments().addAllArguments(expressions)) + .map(Function.Builder::build) + .map(Expression.newBuilder()::setFunction) + .map(Expression.Builder::build); + } + + protected Single transformOrderBy(OrderByExpression orderBy) { + return this.transformExpression(orderBy.getExpression()) + .map(orderBy.toBuilder()::setExpression) + .map(OrderByExpression.Builder::build); + } + + protected Single transformFilter(Filter filter) { + if (filter.equals(Filter.getDefaultInstance())) { + return Single.just(filter); + } + + Single lhsSingle = this.transformExpression(filter.getLhs()); + Single rhsSingle = this.transformExpression(filter.getRhs()); + Single> childFilterListSingle = + Observable.fromIterable(filter.getChildFilterList()) + .concatMapSingle(this::transformFilter) + .toList(); + return zip( + lhsSingle, + rhsSingle, + childFilterListSingle, + (lhs, rhs, childFilterList) -> + this.rebuildFilterOmittingDefaults(filter, lhs, rhs, childFilterList)); + } + + private Single> transformExpressionList(List expressionList) { + return Observable.fromIterable(expressionList) + .concatMapSingle(this::transformExpression) + .toList(); + } + + private Single> transformOrderByList( + List orderByList) { + return Observable.fromIterable(orderByList).concatMapSingle(this::transformOrderBy).toList(); + } + + /** + * This doesn't change any functional behavior, but omits fields that aren't needed, shrinking the + * object and keeping it equivalent to the source object for equality checks. + */ + private Filter rebuildFilterOmittingDefaults( + Filter original, Expression lhs, Expression rhs, List childFilters) { + Filter.Builder builder = original.toBuilder(); + + if (Expression.getDefaultInstance().equals(lhs)) { + builder.clearLhs(); + } else { + builder.setLhs(lhs); + } + + if (Expression.getDefaultInstance().equals(rhs)) { + builder.clearRhs(); + } else { + builder.setRhs(rhs); + } + + return builder.clearChildFilter().addAllChildFilter(childFilters).build(); + } + + private void debugLogIfRequestTransformed(QueryRequest original, QueryRequest transformed) { + if (!original.equals(transformed)) { + getLogger() + .debug( + "Request transformation occurred. Original request: {} Transformed Request: {}", + original, + transformed); + } + } +} diff --git a/query-service-impl/src/main/java/org/hypertrace/core/query/service/QueryRequestUtil.java b/query-service-impl/src/main/java/org/hypertrace/core/query/service/QueryRequestUtil.java index f63db9ee..9bd89207 100644 --- a/query-service-impl/src/main/java/org/hypertrace/core/query/service/QueryRequestUtil.java +++ b/query-service-impl/src/main/java/org/hypertrace/core/query/service/QueryRequestUtil.java @@ -1,12 +1,9 @@ package org.hypertrace.core.query.service; import static org.hypertrace.core.query.service.api.Expression.ValueCase.ATTRIBUTE_EXPRESSION; -import static org.hypertrace.core.query.service.api.Expression.ValueCase.COLUMNIDENTIFIER; -import static org.hypertrace.core.query.service.api.Expression.ValueCase.FUNCTION; import java.util.Optional; import org.hypertrace.core.query.service.api.AttributeExpression; -import org.hypertrace.core.query.service.api.ColumnIdentifier; import org.hypertrace.core.query.service.api.Expression; import org.hypertrace.core.query.service.api.Expression.ValueCase; import org.hypertrace.core.query.service.api.Filter; @@ -21,12 +18,6 @@ */ public class QueryRequestUtil { - public static Expression createColumnExpression(String columnName) { - return Expression.newBuilder() - .setColumnIdentifier(ColumnIdentifier.newBuilder().setColumnName(columnName)) - .build(); - } - public static Expression createStringLiteralExpression(String value) { return Expression.newBuilder() .setLiteral( @@ -145,10 +136,7 @@ public static Optional getAlias(Expression expression) { ? getLogicalColumnName(expression).get() : expression.getColumnIdentifier().getAlias()); case ATTRIBUTE_EXPRESSION: - return Optional.of( - expression.getAttributeExpression().getAlias().isBlank() - ? getLogicalColumnName(expression).get() - : expression.getAttributeExpression().getAlias()); + return Optional.of(getAlias(expression.getAttributeExpression())); case FUNCTION: // todo: handle recursive functions max(rollup(time,50) // workaround is to use alias for now @@ -160,4 +148,10 @@ public static Optional getAlias(Expression expression) { return Optional.empty(); } } + + public static String getAlias(AttributeExpression attributeExpression) { + return attributeExpression.getAlias().isBlank() + ? attributeExpression.getAttributeId() + : attributeExpression.getAlias(); + } } diff --git a/query-service-impl/src/main/java/org/hypertrace/core/query/service/QueryServiceModule.java b/query-service-impl/src/main/java/org/hypertrace/core/query/service/QueryServiceModule.java index 09c5d84e..5bfec847 100644 --- a/query-service-impl/src/main/java/org/hypertrace/core/query/service/QueryServiceModule.java +++ b/query-service-impl/src/main/java/org/hypertrace/core/query/service/QueryServiceModule.java @@ -6,6 +6,7 @@ import javax.inject.Singleton; import org.hypertrace.core.attribute.service.cachingclient.CachingAttributeClient; import org.hypertrace.core.query.service.api.QueryServiceGrpc.QueryServiceImplBase; +import org.hypertrace.core.query.service.attribubteexpression.ExpressionModule; import org.hypertrace.core.query.service.pinot.PinotModule; import org.hypertrace.core.query.service.projection.ProjectionModule; import org.hypertrace.core.query.service.prometheus.PrometheusModule; @@ -33,5 +34,6 @@ protected void configure() { install(new PinotModule()); install(new ProjectionModule()); install(new PrometheusModule()); + install(new ExpressionModule()); } } diff --git a/query-service-impl/src/main/java/org/hypertrace/core/query/service/QueryTransformation.java b/query-service-impl/src/main/java/org/hypertrace/core/query/service/QueryTransformation.java index 09428e0f..0c3d3049 100644 --- a/query-service-impl/src/main/java/org/hypertrace/core/query/service/QueryTransformation.java +++ b/query-service-impl/src/main/java/org/hypertrace/core/query/service/QueryTransformation.java @@ -2,10 +2,20 @@ import io.reactivex.rxjava3.core.Single; import org.hypertrace.core.query.service.api.QueryRequest; +import org.jetbrains.annotations.NotNull; -public interface QueryTransformation { +public interface QueryTransformation extends Comparable { Single transform( QueryRequest queryRequest, QueryTransformationContext transformationContext); + default int getPriority() { + return 10; + } + + @Override + default int compareTo(@NotNull QueryTransformation other) { + return Integer.compare(this.getPriority(), other.getPriority()); + } + interface QueryTransformationContext {} } diff --git a/query-service-impl/src/main/java/org/hypertrace/core/query/service/QueryTransformationPipeline.java b/query-service-impl/src/main/java/org/hypertrace/core/query/service/QueryTransformationPipeline.java index c497ae10..797be9b4 100644 --- a/query-service-impl/src/main/java/org/hypertrace/core/query/service/QueryTransformationPipeline.java +++ b/query-service-impl/src/main/java/org/hypertrace/core/query/service/QueryTransformationPipeline.java @@ -24,6 +24,7 @@ Single transform(QueryRequest originalRequest, String tenantId) { QueryTransformationContext transformationContext = new DefaultQueryTransformationContext(tenantId); return Observable.fromIterable(transformations) + .sorted() .reduce( Single.just(originalRequest), (requestSingle, transformation) -> diff --git a/query-service-impl/src/main/java/org/hypertrace/core/query/service/attribubteexpression/AttributeNormalizationTransformation.java b/query-service-impl/src/main/java/org/hypertrace/core/query/service/attribubteexpression/AttributeNormalizationTransformation.java new file mode 100644 index 00000000..fac3b41b --- /dev/null +++ b/query-service-impl/src/main/java/org/hypertrace/core/query/service/attribubteexpression/AttributeNormalizationTransformation.java @@ -0,0 +1,41 @@ +package org.hypertrace.core.query.service.attribubteexpression; + +import io.reactivex.rxjava3.core.Single; +import lombok.extern.slf4j.Slf4j; +import org.hypertrace.core.query.service.AbstractQueryTransformation; +import org.hypertrace.core.query.service.QueryRequestUtil; +import org.hypertrace.core.query.service.api.ColumnIdentifier; +import org.hypertrace.core.query.service.api.Expression; +import org.slf4j.Logger; + +@Slf4j +final class AttributeNormalizationTransformation extends AbstractQueryTransformation { + + @Override + public int getPriority() { + // Run before default transformations + return 1; + } + + @Override + protected Logger getLogger() { + return log; + } + + @Override + protected Single transformColumnIdentifier(ColumnIdentifier columnIdentifier) { + Expression.Builder expressionBuilder = + QueryRequestUtil.createSimpleAttributeExpression(columnIdentifier.getColumnName()); + if (columnIdentifier.getAlias().isBlank()) { + return Single.just(expressionBuilder.build()); + } + + return Single.just( + expressionBuilder + .setAttributeExpression( + expressionBuilder + .getAttributeExpressionBuilder() + .setAlias(columnIdentifier.getAlias())) + .build()); + } +} diff --git a/query-service-impl/src/main/java/org/hypertrace/core/query/service/attribubteexpression/AttributeSubpathExistsFilteringTransformation.java b/query-service-impl/src/main/java/org/hypertrace/core/query/service/attribubteexpression/AttributeSubpathExistsFilteringTransformation.java new file mode 100644 index 00000000..b1a93900 --- /dev/null +++ b/query-service-impl/src/main/java/org/hypertrace/core/query/service/attribubteexpression/AttributeSubpathExistsFilteringTransformation.java @@ -0,0 +1,133 @@ +package org.hypertrace.core.query.service.attribubteexpression; + +import static org.hypertrace.core.query.service.QueryRequestUtil.createContainsKeyFilter; +import static org.hypertrace.core.query.service.QueryRequestUtil.isAttributeExpressionWithSubpath; + +import com.google.common.collect.ImmutableSet; +import java.util.List; +import java.util.Optional; +import java.util.Set; +import java.util.function.Predicate; +import java.util.stream.Collectors; +import java.util.stream.Stream; +import lombok.extern.slf4j.Slf4j; +import org.hypertrace.core.query.service.AbstractQueryTransformation; +import org.hypertrace.core.query.service.QueryRequestUtil; +import org.hypertrace.core.query.service.api.AttributeExpression; +import org.hypertrace.core.query.service.api.Expression; +import org.hypertrace.core.query.service.api.Filter; +import org.hypertrace.core.query.service.api.Operator; +import org.hypertrace.core.query.service.api.OrderByExpression; +import org.hypertrace.core.query.service.api.QueryRequest; +import org.slf4j.Logger; + +@Slf4j +final class AttributeSubpathExistsFilteringTransformation extends AbstractQueryTransformation { + + @Override + protected Logger getLogger() { + return log; + } + + @Override + protected QueryRequest rebuildRequest( + QueryRequest original, + List selections, + List aggregations, + Filter originalFilter, + List groupBys, + List orderBys) { + Filter updatedFilter = + rebuildFilterForComplexAttributeExpression(originalFilter, orderBys, selections) + .orElse(originalFilter); + + return super.rebuildRequest( + original, selections, aggregations, updatedFilter, groupBys, orderBys); + } + + /* + * We need the CONTAINS_KEY filter in all filters and order bys dealing with complex + * attribute expressions as Pinot gives error if particular key is absent. Rest all work fine. + * To handle order bys, we add the corresponding filter at the top and 'AND' it with the main filter. + * To handle filter, we modify each filter (say filter1) as : "CONTAINS_KEY AND filter1". + */ + private Optional rebuildFilterForComplexAttributeExpression( + Filter originalFilter, List orderBys, List selections) { + + Set rootFilters = + ImmutableSet.builder() + .add(updateFilterForComplexAttributeExpressionFromFilter(originalFilter)) + .addAll(createFilterForComplexAttributeExpressionFromOrderBy(orderBys)) + .addAll(createFilterForComplexAttributeExpressionFromSelection(selections)) + .build() + .stream() + .filter(Predicate.not(Filter.getDefaultInstance()::equals)) + .collect(Collectors.toUnmodifiableSet()); + + if (rootFilters.isEmpty()) { + return Optional.empty(); + } + + if (rootFilters.size() == 1) { + return rootFilters.stream().findFirst(); + } + return Optional.of( + Filter.newBuilder().setOperator(Operator.AND).addAllChildFilter(rootFilters).build()); + } + + private Filter updateFilterForComplexAttributeExpressionFromFilter(Filter originalFilter) { + /* + * If childFilter is present, then the expected operators comprise the logical operators. + * If childFilter is absent, then the filter is a leaf filter which will have lhs and rhs. + */ + if (originalFilter.getChildFilterCount() > 0) { + Filter.Builder builder = Filter.newBuilder(); + builder.setOperator(originalFilter.getOperator()); + originalFilter + .getChildFilterList() + .forEach( + childFilter -> + builder.addChildFilter( + updateFilterForComplexAttributeExpressionFromFilter(childFilter))); + return builder.build(); + } + if (isAttributeExpressionWithSubpath(originalFilter.getLhs())) { + Filter childFilter = + createContainsKeyFilter(originalFilter.getLhs().getAttributeExpression()); + return Filter.newBuilder() + .setOperator(Operator.AND) + .addChildFilter(originalFilter) + .addChildFilter(childFilter) + .build(); + } + return originalFilter; + } + + private List createFilterForComplexAttributeExpressionFromOrderBy( + List orderByExpressionList) { + return orderByExpressionList.stream() + .map(OrderByExpression::getExpression) + .filter(QueryRequestUtil::isAttributeExpressionWithSubpath) + .map(Expression::getAttributeExpression) + .map(QueryRequestUtil::createContainsKeyFilter) + .collect(Collectors.toList()); + } + + private List createFilterForComplexAttributeExpressionFromSelection( + List expressions) { + return expressions.stream() + .flatMap(this::getAnyAttributeExpression) + .map(QueryRequestUtil::createContainsKeyFilter) + .collect(Collectors.toList()); + } + + private Stream getAnyAttributeExpression(Expression expression) { + if (expression.hasFunction()) { + return expression.getFunction().getArgumentsList().stream() + .flatMap(this::getAnyAttributeExpression); + } + return Stream.of(expression) + .filter(QueryRequestUtil::isAttributeExpressionWithSubpath) + .map(Expression::getAttributeExpression); + } +} diff --git a/query-service-impl/src/main/java/org/hypertrace/core/query/service/attribubteexpression/ExpressionModule.java b/query-service-impl/src/main/java/org/hypertrace/core/query/service/attribubteexpression/ExpressionModule.java new file mode 100644 index 00000000..8fdcdeba --- /dev/null +++ b/query-service-impl/src/main/java/org/hypertrace/core/query/service/attribubteexpression/ExpressionModule.java @@ -0,0 +1,16 @@ +package org.hypertrace.core.query.service.attribubteexpression; + +import com.google.inject.AbstractModule; +import com.google.inject.multibindings.Multibinder; +import org.hypertrace.core.query.service.QueryTransformation; + +public class ExpressionModule extends AbstractModule { + + @Override + protected void configure() { + Multibinder transformationMultibinder = + Multibinder.newSetBinder(binder(), QueryTransformation.class); + transformationMultibinder.addBinding().to(AttributeSubpathExistsFilteringTransformation.class); + transformationMultibinder.addBinding().to(AttributeNormalizationTransformation.class); + } +} diff --git a/query-service-impl/src/main/java/org/hypertrace/core/query/service/projection/ProjectionTransformation.java b/query-service-impl/src/main/java/org/hypertrace/core/query/service/projection/ProjectionTransformation.java index f1e9b819..1536dfe8 100644 --- a/query-service-impl/src/main/java/org/hypertrace/core/query/service/projection/ProjectionTransformation.java +++ b/query-service-impl/src/main/java/org/hypertrace/core/query/service/projection/ProjectionTransformation.java @@ -2,23 +2,20 @@ import static io.reactivex.rxjava3.core.Single.zip; import static org.hypertrace.core.query.service.QueryRequestUtil.createBooleanLiteralExpression; -import static org.hypertrace.core.query.service.QueryRequestUtil.createColumnExpression; -import static org.hypertrace.core.query.service.QueryRequestUtil.createContainsKeyFilter; import static org.hypertrace.core.query.service.QueryRequestUtil.createDoubleLiteralExpression; import static org.hypertrace.core.query.service.QueryRequestUtil.createLongLiteralExpression; import static org.hypertrace.core.query.service.QueryRequestUtil.createNullNumberLiteralExpression; import static org.hypertrace.core.query.service.QueryRequestUtil.createNullStringLiteralExpression; +import static org.hypertrace.core.query.service.QueryRequestUtil.createSimpleAttributeExpression; import static org.hypertrace.core.query.service.QueryRequestUtil.createStringLiteralExpression; -import static org.hypertrace.core.query.service.QueryRequestUtil.isAttributeExpressionWithSubpath; import io.reactivex.rxjava3.core.Maybe; import io.reactivex.rxjava3.core.Observable; import io.reactivex.rxjava3.core.Single; import java.util.List; import java.util.Optional; -import java.util.stream.Collectors; -import java.util.stream.Stream; import javax.inject.Inject; +import lombok.extern.slf4j.Slf4j; import org.hypertrace.core.attribute.service.cachingclient.CachingAttributeClient; import org.hypertrace.core.attribute.service.projection.AttributeProjection; import org.hypertrace.core.attribute.service.projection.AttributeProjectionRegistry; @@ -28,23 +25,16 @@ import org.hypertrace.core.attribute.service.v1.Projection; import org.hypertrace.core.attribute.service.v1.ProjectionExpression; import org.hypertrace.core.attribute.service.v1.ProjectionOperator; +import org.hypertrace.core.query.service.AbstractQueryTransformation; import org.hypertrace.core.query.service.QueryFunctionConstants; import org.hypertrace.core.query.service.QueryRequestUtil; -import org.hypertrace.core.query.service.QueryTransformation; import org.hypertrace.core.query.service.api.AttributeExpression; -import org.hypertrace.core.query.service.api.ColumnIdentifier; import org.hypertrace.core.query.service.api.Expression; -import org.hypertrace.core.query.service.api.Filter; import org.hypertrace.core.query.service.api.Function; -import org.hypertrace.core.query.service.api.Operator; -import org.hypertrace.core.query.service.api.OrderByExpression; -import org.hypertrace.core.query.service.api.QueryRequest; import org.slf4j.Logger; -import org.slf4j.LoggerFactory; -final class ProjectionTransformation implements QueryTransformation { - - private static final Logger LOG = LoggerFactory.getLogger(ProjectionTransformation.class); +@Slf4j +final class ProjectionTransformation extends AbstractQueryTransformation { private final CachingAttributeClient attributeClient; private final AttributeProjectionRegistry projectionRegistry; @@ -57,96 +47,33 @@ final class ProjectionTransformation implements QueryTransformation { } @Override - public Single transform( - QueryRequest queryRequest, QueryTransformationContext transformationContext) { - return zip( - this.transformExpressionList(queryRequest.getSelectionList()), - this.transformExpressionList(queryRequest.getAggregationList()), - this.transformFilter(queryRequest.getFilter()), - this.transformExpressionList(queryRequest.getGroupByList()), - this.transformOrderByList(queryRequest.getOrderByList()), - (selections, aggregations, filter, groupBys, orderBys) -> - this.rebuildRequest( - queryRequest, selections, aggregations, filter, groupBys, orderBys)) - .doOnSuccess(transformed -> this.debugLogIfRequestTransformed(queryRequest, transformed)); + protected Logger getLogger() { + return log; } - private Single> transformExpressionList(List expressionList) { - return Observable.fromIterable(expressionList) - .concatMapSingle(this::transformExpression) - .toList(); - } - - private Single transformExpression(Expression expression) { - switch (expression.getValueCase()) { - case COLUMNIDENTIFIER: - return this.transformColumnIdentifier(expression.getColumnIdentifier()); - case ATTRIBUTE_EXPRESSION: - return this.transformAttributeExpression(expression.getAttributeExpression()); - case FUNCTION: - return this.transformFunction(expression.getFunction()) - .map(expression.toBuilder()::setFunction) - .map(Expression.Builder::build); - case ORDERBY: - return this.transformOrderBy(expression.getOrderBy()) - .map(expression.toBuilder()::setOrderBy) - .map(Expression.Builder::build); - case LITERAL: - case VALUE_NOT_SET: - default: - return Single.just(expression); - } - } - - private Single transformColumnIdentifier(ColumnIdentifier columnIdentifier) { - return this.projectAttributeIfPossible(columnIdentifier.getColumnName()) - .map(expression -> this.aliasToMatchOriginal(getOriginalKey(columnIdentifier), expression)) - .defaultIfEmpty(Expression.newBuilder().setColumnIdentifier(columnIdentifier).build()); - } - - private Single transformAttributeExpression(AttributeExpression attributeExpression) { + @Override + protected Single transformAttributeExpression( + AttributeExpression attributeExpression) { return this.projectAttributeIfPossible(attributeExpression.getAttributeId()) .map( - expression -> - this.aliasToMatchOriginal(getOriginalKey(attributeExpression), expression)) + projectedExpression -> + attributeExpression.hasSubpath() + ? this.addSubpathOrThrow(projectedExpression, attributeExpression.getSubpath()) + : projectedExpression) + .map(expression -> this.aliasToMatchOriginal(attributeExpression, expression)) .defaultIfEmpty( Expression.newBuilder().setAttributeExpression(attributeExpression).build()); } - private Single transformFunction(Function function) { - return this.transformExpressionList(function.getArgumentsList()) - .map(expressions -> function.toBuilder().clearArguments().addAllArguments(expressions)) - .map(Function.Builder::build); - } - - private Single> transformOrderByList( - List orderByList) { - return Observable.fromIterable(orderByList).concatMapSingle(this::transformOrderBy).toList(); - } - - private Single transformOrderBy(OrderByExpression orderBy) { - return this.transformExpression(orderBy.getExpression()) - .map(orderBy.toBuilder()::setExpression) - .map(OrderByExpression.Builder::build); - } - - private Single transformFilter(Filter filter) { - if (filter.equals(Filter.getDefaultInstance())) { - return Single.just(filter); + private Expression addSubpathOrThrow(Expression projectedExpression, String subpath) { + if (!QueryRequestUtil.isSimpleAttributeExpression(projectedExpression)) { + throw new IllegalArgumentException( + "Cannot use subpath for expression with non-trivial projection: " + projectedExpression); } - - Single lhsSingle = this.transformExpression(filter.getLhs()); - Single rhsSingle = this.transformExpression(filter.getRhs()); - Single> childFilterListSingle = - Observable.fromIterable(filter.getChildFilterList()) - .concatMapSingle(this::transformFilter) - .toList(); - return zip( - lhsSingle, - rhsSingle, - childFilterListSingle, - (lhs, rhs, childFilterList) -> - this.rebuildFilterOmittingDefaults(filter, lhs, rhs, childFilterList)); + return Expression.newBuilder() + .setAttributeExpression( + projectedExpression.getAttributeExpression().toBuilder().setSubpath(subpath)) + .build(); } private Maybe projectAttributeIfPossible(String attributeId) { @@ -172,7 +99,8 @@ private Single rewriteProjectionAsQueryExpression( Projection projection, AttributeKind expectedType) { switch (projection.getValueCase()) { case ATTRIBUTE_ID: - return this.transformExpression(createColumnExpression(projection.getAttributeId())); + return this.transformExpression( + createSimpleAttributeExpression(projection.getAttributeId()).build()); case LITERAL: return this.rewriteLiteralAsQueryExpression(projection.getLiteral(), expectedType); case EXPRESSION: @@ -280,13 +208,9 @@ private Single convertOperator(ProjectionOperator operator) { } } - private Expression aliasToMatchOriginal(String originalKey, Expression newExpression) { + private Expression aliasToMatchOriginal(AttributeExpression original, Expression newExpression) { + String originalKey = QueryRequestUtil.getAlias(original); switch (newExpression.getValueCase()) { - case COLUMNIDENTIFIER: - return newExpression.toBuilder() - .setColumnIdentifier( - newExpression.getColumnIdentifier().toBuilder().setAlias(originalKey)) - .build(); case ATTRIBUTE_EXPRESSION: return newExpression.toBuilder() .setAttributeExpression( @@ -303,166 +227,4 @@ private Expression aliasToMatchOriginal(String originalKey, Expression newExpres return newExpression; } } - - private void debugLogIfRequestTransformed(QueryRequest original, QueryRequest transformed) { - if (!original.equals(transformed)) { - LOG.debug( - "Request transformation occurred. Original request: {} Transformed Request: {}", - original, - transformed); - } - } - - /** - * This doesn't change any functional behavior, but omits fields that aren't needed, shrinking the - * object and keeping it equivalent to the source object for equality checks. - */ - private Filter rebuildFilterOmittingDefaults( - Filter original, Expression lhs, Expression rhs, List childFilters) { - Filter.Builder builder = original.toBuilder(); - - if (Expression.getDefaultInstance().equals(lhs)) { - builder.clearLhs(); - } else { - builder.setLhs(lhs); - } - - if (Expression.getDefaultInstance().equals(rhs)) { - builder.clearRhs(); - } else { - builder.setRhs(rhs); - } - - return builder.clearChildFilter().addAllChildFilter(childFilters).build(); - } - - private QueryRequest rebuildRequest( - QueryRequest original, - List selections, - List aggregations, - Filter originalFilter, - List groupBys, - List orderBys) { - - QueryRequest.Builder builder = original.toBuilder(); - Filter updatedFilter = - rebuildFilterForComplexAttributeExpression(originalFilter, orderBys, selections); - - if (Filter.getDefaultInstance().equals(updatedFilter)) { - builder.clearFilter(); - } else { - builder.setFilter(updatedFilter); - } - - return builder - .clearSelection() - .addAllSelection(selections) - .clearAggregation() - .addAllAggregation(aggregations) - .clearGroupBy() - .addAllGroupBy(groupBys) - .clearOrderBy() - .addAllOrderBy(orderBys) - .build(); - } - - /* - * We need the CONTAINS_KEY filter in all filters, selections and order bys dealing with complex - * attribute expressions as Pinot gives error if particular key is absent. Rest all work fine. - * To handle order bys and selections, we add the corresponding filter at the top and 'AND' it with the main filter. - * To handle filter, we modify each filter (say filter1) as : "CONTAINS_KEY AND filter1". - */ - private Filter rebuildFilterForComplexAttributeExpression( - Filter originalFilter, List orderBys, List selections) { - - Filter updatedFilter = updateFilterForComplexAttributeExpressionFromFilter(originalFilter); - List filterList = - Stream.concat( - createFilterForComplexAttributeExpressionFromOrderBy(orderBys), - createFilterForComplexAttributeExpressionFromSelection(selections)) - .collect(Collectors.toList()); - - if (filterList.isEmpty()) { - return updatedFilter; - } - - if (!updatedFilter.equals(Filter.getDefaultInstance())) { - return Filter.newBuilder() - .setOperator(Operator.AND) - .addChildFilter(updatedFilter) - .addAllChildFilter(filterList) - .build(); - } - - if (filterList.size() > 1) { - return Filter.newBuilder().setOperator(Operator.AND).addAllChildFilter(filterList).build(); - } else { - return filterList.get(0); - } - } - - private Filter updateFilterForComplexAttributeExpressionFromFilter(Filter originalFilter) { - /* - * If childFilter is present, then the expected operators comprise the logical operators. - * If childFilter is absent, then the filter is a leaf filter which will have lhs and rhs. - */ - if (originalFilter.getChildFilterCount() > 0) { - Filter.Builder builder = Filter.newBuilder(); - builder.setOperator(originalFilter.getOperator()); - originalFilter - .getChildFilterList() - .forEach( - childFilter -> - builder.addChildFilter( - updateFilterForComplexAttributeExpressionFromFilter(childFilter))); - return builder.build(); - } else if (isAttributeExpressionWithSubpath(originalFilter.getLhs())) { - Filter childFilter = - createContainsKeyFilter(originalFilter.getLhs().getAttributeExpression()); - return Filter.newBuilder() - .setOperator(Operator.AND) - .addChildFilter(originalFilter) - .addChildFilter(childFilter) - .build(); - } else { - return originalFilter; - } - } - - private Stream createFilterForComplexAttributeExpressionFromOrderBy( - List orderByExpressionList) { - return orderByExpressionList.stream() - .map(OrderByExpression::getExpression) - .filter(QueryRequestUtil::isAttributeExpressionWithSubpath) - .map(Expression::getAttributeExpression) - .map(QueryRequestUtil::createContainsKeyFilter); - } - - private Stream createFilterForComplexAttributeExpressionFromSelection( - List selections) { - return selections.stream() - .flatMap(this::getAnyAttributeExpression) - .map(QueryRequestUtil::createContainsKeyFilter); - } - - private Stream getAnyAttributeExpression(Expression selection) { - if (selection.hasFunction()) { - return selection.getFunction().getArgumentsList().stream() - .flatMap(this::getAnyAttributeExpression); - } else { - return Stream.of(selection) - .filter(QueryRequestUtil::isAttributeExpressionWithSubpath) - .map(Expression::getAttributeExpression); - } - } - - private String getOriginalKey(AttributeExpression attributeExpression) { - String alias = attributeExpression.getAlias(); - return alias.isEmpty() ? attributeExpression.getAttributeId() : alias; - } - - private String getOriginalKey(ColumnIdentifier columnIdentifier) { - String alias = columnIdentifier.getAlias(); - return alias.isEmpty() ? columnIdentifier.getColumnName() : alias; - } } From 3a35f1c1f5ff4a936a8bbfdeff9125b51b616775 Mon Sep 17 00:00:00 2001 From: Aaron Steinfeld Date: Thu, 23 Dec 2021 12:57:44 -0500 Subject: [PATCH 18/23] test: update tests --- .../service/AbstractQueryTransformation.java | 11 +- .../query/service/QueryServiceModule.java | 4 +- ...le.java => AttributeExpressionModule.java} | 8 +- ...xpressionNormalizationTransformation.java} | 2 +- ...SubpathExistsFilteringTransformation.java} | 5 +- .../service/QueryRequestBuilderUtils.java | 7 + .../QueryTransformationPipelineTest.java | 6 +- ...essionNormalizationTransformationTest.java | 75 ++++ ...pathExistsFilteringTransformationTest.java | 235 ++++++++++++ .../ProjectionTransformationTest.java | 345 +++--------------- 10 files changed, 400 insertions(+), 298 deletions(-) rename query-service-impl/src/main/java/org/hypertrace/core/query/service/attribubteexpression/{ExpressionModule.java => AttributeExpressionModule.java} (58%) rename query-service-impl/src/main/java/org/hypertrace/core/query/service/attribubteexpression/{AttributeNormalizationTransformation.java => AttributeExpressionNormalizationTransformation.java} (92%) rename query-service-impl/src/main/java/org/hypertrace/core/query/service/attribubteexpression/{AttributeSubpathExistsFilteringTransformation.java => AttributeExpressionSubpathExistsFilteringTransformation.java} (96%) create mode 100644 query-service-impl/src/test/java/org/hypertrace/core/query/service/attribubteexpression/AttributeExpressionNormalizationTransformationTest.java create mode 100644 query-service-impl/src/test/java/org/hypertrace/core/query/service/attribubteexpression/AttributeExpressionSubpathExistsFilteringTransformationTest.java diff --git a/query-service-impl/src/main/java/org/hypertrace/core/query/service/AbstractQueryTransformation.java b/query-service-impl/src/main/java/org/hypertrace/core/query/service/AbstractQueryTransformation.java index 73434198..fe955ccd 100644 --- a/query-service-impl/src/main/java/org/hypertrace/core/query/service/AbstractQueryTransformation.java +++ b/query-service-impl/src/main/java/org/hypertrace/core/query/service/AbstractQueryTransformation.java @@ -42,7 +42,15 @@ protected QueryRequest rebuildRequest( List groupBys, List orderBys) { - return original.toBuilder() + QueryRequest.Builder builder = original.toBuilder(); + + if (Filter.getDefaultInstance().equals(filter)) { + builder.clearFilter(); + } else { + builder.setFilter(filter); + } + + return builder .clearSelection() .addAllSelection(selections) .clearAggregation() @@ -51,7 +59,6 @@ protected QueryRequest rebuildRequest( .addAllGroupBy(groupBys) .clearOrderBy() .addAllOrderBy(orderBys) - .setFilter(filter) .build(); } diff --git a/query-service-impl/src/main/java/org/hypertrace/core/query/service/QueryServiceModule.java b/query-service-impl/src/main/java/org/hypertrace/core/query/service/QueryServiceModule.java index 5bfec847..9fb4c010 100644 --- a/query-service-impl/src/main/java/org/hypertrace/core/query/service/QueryServiceModule.java +++ b/query-service-impl/src/main/java/org/hypertrace/core/query/service/QueryServiceModule.java @@ -6,7 +6,7 @@ import javax.inject.Singleton; import org.hypertrace.core.attribute.service.cachingclient.CachingAttributeClient; import org.hypertrace.core.query.service.api.QueryServiceGrpc.QueryServiceImplBase; -import org.hypertrace.core.query.service.attribubteexpression.ExpressionModule; +import org.hypertrace.core.query.service.attribubteexpression.AttributeExpressionModule; import org.hypertrace.core.query.service.pinot.PinotModule; import org.hypertrace.core.query.service.projection.ProjectionModule; import org.hypertrace.core.query.service.prometheus.PrometheusModule; @@ -34,6 +34,6 @@ protected void configure() { install(new PinotModule()); install(new ProjectionModule()); install(new PrometheusModule()); - install(new ExpressionModule()); + install(new AttributeExpressionModule()); } } diff --git a/query-service-impl/src/main/java/org/hypertrace/core/query/service/attribubteexpression/ExpressionModule.java b/query-service-impl/src/main/java/org/hypertrace/core/query/service/attribubteexpression/AttributeExpressionModule.java similarity index 58% rename from query-service-impl/src/main/java/org/hypertrace/core/query/service/attribubteexpression/ExpressionModule.java rename to query-service-impl/src/main/java/org/hypertrace/core/query/service/attribubteexpression/AttributeExpressionModule.java index 8fdcdeba..6cc3e929 100644 --- a/query-service-impl/src/main/java/org/hypertrace/core/query/service/attribubteexpression/ExpressionModule.java +++ b/query-service-impl/src/main/java/org/hypertrace/core/query/service/attribubteexpression/AttributeExpressionModule.java @@ -4,13 +4,15 @@ import com.google.inject.multibindings.Multibinder; import org.hypertrace.core.query.service.QueryTransformation; -public class ExpressionModule extends AbstractModule { +public class AttributeExpressionModule extends AbstractModule { @Override protected void configure() { Multibinder transformationMultibinder = Multibinder.newSetBinder(binder(), QueryTransformation.class); - transformationMultibinder.addBinding().to(AttributeSubpathExistsFilteringTransformation.class); - transformationMultibinder.addBinding().to(AttributeNormalizationTransformation.class); + transformationMultibinder + .addBinding() + .to(AttributeExpressionSubpathExistsFilteringTransformation.class); + transformationMultibinder.addBinding().to(AttributeExpressionNormalizationTransformation.class); } } diff --git a/query-service-impl/src/main/java/org/hypertrace/core/query/service/attribubteexpression/AttributeNormalizationTransformation.java b/query-service-impl/src/main/java/org/hypertrace/core/query/service/attribubteexpression/AttributeExpressionNormalizationTransformation.java similarity index 92% rename from query-service-impl/src/main/java/org/hypertrace/core/query/service/attribubteexpression/AttributeNormalizationTransformation.java rename to query-service-impl/src/main/java/org/hypertrace/core/query/service/attribubteexpression/AttributeExpressionNormalizationTransformation.java index fac3b41b..10cabf32 100644 --- a/query-service-impl/src/main/java/org/hypertrace/core/query/service/attribubteexpression/AttributeNormalizationTransformation.java +++ b/query-service-impl/src/main/java/org/hypertrace/core/query/service/attribubteexpression/AttributeExpressionNormalizationTransformation.java @@ -9,7 +9,7 @@ import org.slf4j.Logger; @Slf4j -final class AttributeNormalizationTransformation extends AbstractQueryTransformation { +final class AttributeExpressionNormalizationTransformation extends AbstractQueryTransformation { @Override public int getPriority() { diff --git a/query-service-impl/src/main/java/org/hypertrace/core/query/service/attribubteexpression/AttributeSubpathExistsFilteringTransformation.java b/query-service-impl/src/main/java/org/hypertrace/core/query/service/attribubteexpression/AttributeExpressionSubpathExistsFilteringTransformation.java similarity index 96% rename from query-service-impl/src/main/java/org/hypertrace/core/query/service/attribubteexpression/AttributeSubpathExistsFilteringTransformation.java rename to query-service-impl/src/main/java/org/hypertrace/core/query/service/attribubteexpression/AttributeExpressionSubpathExistsFilteringTransformation.java index b1a93900..eccce03d 100644 --- a/query-service-impl/src/main/java/org/hypertrace/core/query/service/attribubteexpression/AttributeSubpathExistsFilteringTransformation.java +++ b/query-service-impl/src/main/java/org/hypertrace/core/query/service/attribubteexpression/AttributeExpressionSubpathExistsFilteringTransformation.java @@ -22,7 +22,8 @@ import org.slf4j.Logger; @Slf4j -final class AttributeSubpathExistsFilteringTransformation extends AbstractQueryTransformation { +final class AttributeExpressionSubpathExistsFilteringTransformation + extends AbstractQueryTransformation { @Override protected Logger getLogger() { @@ -62,7 +63,7 @@ private Optional rebuildFilterForComplexAttributeExpression( .build() .stream() .filter(Predicate.not(Filter.getDefaultInstance()::equals)) - .collect(Collectors.toUnmodifiableSet()); + .collect(ImmutableSet.toImmutableSet()); if (rootFilters.isEmpty()) { return Optional.empty(); diff --git a/query-service-impl/src/test/java/org/hypertrace/core/query/service/QueryRequestBuilderUtils.java b/query-service-impl/src/test/java/org/hypertrace/core/query/service/QueryRequestBuilderUtils.java index 51b739e1..84ac05da 100644 --- a/query-service-impl/src/test/java/org/hypertrace/core/query/service/QueryRequestBuilderUtils.java +++ b/query-service-impl/src/test/java/org/hypertrace/core/query/service/QueryRequestBuilderUtils.java @@ -37,6 +37,13 @@ public static Expression createAliasedColumnExpression(String columnName, String .build(); } + public static Expression createAliasedAttributeExpression(String columnName, String alias) { + return Expression.newBuilder() + .setAttributeExpression( + AttributeExpression.newBuilder().setAttributeId(columnName).setAlias(alias)) + .build(); + } + public static Expression createCountByColumnSelection(String columnNames) { return createFunctionExpression("Count", createColumnExpression(columnNames).build()); } diff --git a/query-service-impl/src/test/java/org/hypertrace/core/query/service/QueryTransformationPipelineTest.java b/query-service-impl/src/test/java/org/hypertrace/core/query/service/QueryTransformationPipelineTest.java index 238b630e..49448eb9 100644 --- a/query-service-impl/src/test/java/org/hypertrace/core/query/service/QueryTransformationPipelineTest.java +++ b/query-service-impl/src/test/java/org/hypertrace/core/query/service/QueryTransformationPipelineTest.java @@ -44,10 +44,14 @@ void callsEachTransformationWithPreviousResult() { when(secondTransformation.transform(same(firstResult), any(QueryTransformationContext.class))) .thenReturn(Single.just(secondResult)); + when(firstTransformation.compareTo(any())).thenCallRealMethod(); + when(firstTransformation.getPriority()).thenReturn(1); + when(secondTransformation.getPriority()).thenReturn(10); + // purposely flip order to make sure sorted assertSame( secondResult, new QueryTransformationPipeline( - new LinkedHashSet<>(List.of(firstTransformation, secondTransformation))) + new LinkedHashSet<>(List.of(secondTransformation, firstTransformation))) .transform(this.originalRequest, TEST_TENANT_ID) .blockingGet()); } diff --git a/query-service-impl/src/test/java/org/hypertrace/core/query/service/attribubteexpression/AttributeExpressionNormalizationTransformationTest.java b/query-service-impl/src/test/java/org/hypertrace/core/query/service/attribubteexpression/AttributeExpressionNormalizationTransformationTest.java new file mode 100644 index 00000000..5ac7975b --- /dev/null +++ b/query-service-impl/src/test/java/org/hypertrace/core/query/service/attribubteexpression/AttributeExpressionNormalizationTransformationTest.java @@ -0,0 +1,75 @@ +package org.hypertrace.core.query.service.attribubteexpression; + +import static org.hypertrace.core.query.service.QueryRequestBuilderUtils.createAliasedAttributeExpression; +import static org.hypertrace.core.query.service.QueryRequestBuilderUtils.createAliasedColumnExpression; +import static org.hypertrace.core.query.service.QueryRequestBuilderUtils.createAliasedFunctionExpression; +import static org.hypertrace.core.query.service.QueryRequestBuilderUtils.createColumnExpression; +import static org.hypertrace.core.query.service.QueryRequestUtil.createSimpleAttributeExpression; +import static org.hypertrace.core.query.service.QueryRequestUtil.createStringLiteralValueExpression; +import static org.junit.jupiter.api.Assertions.assertEquals; + +import org.hypertrace.core.query.service.QueryFunctionConstants; +import org.hypertrace.core.query.service.QueryTransformation.QueryTransformationContext; +import org.hypertrace.core.query.service.api.Filter; +import org.hypertrace.core.query.service.api.Operator; +import org.hypertrace.core.query.service.api.OrderByExpression; +import org.hypertrace.core.query.service.api.QueryRequest; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; +import org.mockito.Mock; +import org.mockito.junit.jupiter.MockitoExtension; + +@ExtendWith(MockitoExtension.class) +class AttributeExpressionNormalizationTransformationTest { + + @Mock QueryTransformationContext mockTransformationContext; + + private final AttributeExpressionNormalizationTransformation transformation = + new AttributeExpressionNormalizationTransformation(); + + @Test + void columnInAllPositions() { + QueryRequest originalRequest = + QueryRequest.newBuilder() + .addSelection(createAliasedColumnExpression("select-col", "select-alias")) + .addAggregation( + createAliasedFunctionExpression( + QueryFunctionConstants.QUERY_FUNCTION_SUM, + "agg-alias", + createColumnExpression("agg-col").build())) + .setFilter( + Filter.newBuilder() + .setLhs(createAliasedColumnExpression("filter-col", "filter-alias")) + .setOperator(Operator.EQ) + .setRhs(createStringLiteralValueExpression("val"))) + .addOrderBy( + OrderByExpression.newBuilder() + .setExpression(createAliasedColumnExpression("orderby-col", "orderby-alias"))) + .addGroupBy(createAliasedColumnExpression("groupby-col", "groupby-alias")) + .build(); + + QueryRequest expectedTransform = + QueryRequest.newBuilder() + .addSelection(createAliasedAttributeExpression("select-col", "select-alias")) + .addAggregation( + createAliasedFunctionExpression( + QueryFunctionConstants.QUERY_FUNCTION_SUM, + "agg-alias", + createSimpleAttributeExpression("agg-col").build())) + .setFilter( + Filter.newBuilder() + .setLhs(createAliasedAttributeExpression("filter-col", "filter-alias")) + .setOperator(Operator.EQ) + .setRhs(createStringLiteralValueExpression("val"))) + .addOrderBy( + OrderByExpression.newBuilder() + .setExpression( + createAliasedAttributeExpression("orderby-col", "orderby-alias"))) + .addGroupBy(createAliasedAttributeExpression("groupby-col", "groupby-alias")) + .build(); + + assertEquals( + expectedTransform, + this.transformation.transform(originalRequest, mockTransformationContext).blockingGet()); + } +} diff --git a/query-service-impl/src/test/java/org/hypertrace/core/query/service/attribubteexpression/AttributeExpressionSubpathExistsFilteringTransformationTest.java b/query-service-impl/src/test/java/org/hypertrace/core/query/service/attribubteexpression/AttributeExpressionSubpathExistsFilteringTransformationTest.java new file mode 100644 index 00000000..f9a43702 --- /dev/null +++ b/query-service-impl/src/test/java/org/hypertrace/core/query/service/attribubteexpression/AttributeExpressionSubpathExistsFilteringTransformationTest.java @@ -0,0 +1,235 @@ +package org.hypertrace.core.query.service.attribubteexpression; + +import static org.hypertrace.core.query.service.QueryRequestBuilderUtils.createColumnExpression; +import static org.hypertrace.core.query.service.QueryRequestBuilderUtils.createComplexAttributeExpression; +import static org.hypertrace.core.query.service.QueryRequestBuilderUtils.createCompositeFilter; +import static org.hypertrace.core.query.service.QueryRequestBuilderUtils.createEqualsFilter; +import static org.hypertrace.core.query.service.QueryRequestBuilderUtils.createOrderByExpression; +import static org.hypertrace.core.query.service.QueryRequestUtil.createContainsKeyFilter; +import static org.hypertrace.core.query.service.QueryRequestUtil.createStringLiteralValueExpression; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import java.util.List; +import org.hypertrace.core.query.service.QueryTransformation.QueryTransformationContext; +import org.hypertrace.core.query.service.api.Expression; +import org.hypertrace.core.query.service.api.Filter; +import org.hypertrace.core.query.service.api.Operator; +import org.hypertrace.core.query.service.api.QueryRequest; +import org.hypertrace.core.query.service.api.SortOrder; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; +import org.mockito.Mock; +import org.mockito.junit.jupiter.MockitoExtension; + +@ExtendWith(MockitoExtension.class) +class AttributeExpressionSubpathExistsFilteringTransformationTest { + + @Mock QueryTransformationContext mockTransformationContext; + + private final AttributeExpressionSubpathExistsFilteringTransformation transformation = + new AttributeExpressionSubpathExistsFilteringTransformation(); + + @Test + void transQueryWithComplexAttributeExpression_SingleFilter() { + Expression spanTags = createComplexAttributeExpression("Span.tags", "span.kind").build(); + Filter filter = + Filter.newBuilder() + .setLhs(spanTags) + .setOperator(Operator.EQ) + .setRhs(createColumnExpression("server")) + .build(); + QueryRequest originalRequest = QueryRequest.newBuilder().setFilter(filter).build(); + + QueryRequest expectedTransform = + QueryRequest.newBuilder() + .setFilter( + Filter.newBuilder() + .setOperator(Operator.AND) + .addAllChildFilter( + List.of(filter, createContainsKeyFilter(spanTags.getAttributeExpression()))) + .build()) + .build(); + + assertEquals( + expectedTransform, + this.transformation.transform(originalRequest, mockTransformationContext).blockingGet()); + } + + @Test + void transQueryWithComplexAttributeExpression_MultipleFilter() { + Expression spanTags1 = createComplexAttributeExpression("Span.tags", "FLAGS").build(); + Expression spanTags2 = createComplexAttributeExpression("Span.tags", "span.kind").build(); + + Filter childFilter1 = + Filter.newBuilder() + .setLhs(spanTags1) + .setOperator(Operator.EQ) + .setRhs(createStringLiteralValueExpression("0")) + .build(); + Filter childFilter2 = + Filter.newBuilder() + .setLhs(spanTags2) + .setOperator(Operator.EQ) + .setRhs(createStringLiteralValueExpression("server")) + .build(); + + Filter.Builder filter = createCompositeFilter(Operator.AND, childFilter1, childFilter2); + QueryRequest originalRequest = QueryRequest.newBuilder().setFilter(filter).build(); + + QueryRequest expectedTransform = + this.transformation.transform(originalRequest, mockTransformationContext).blockingGet(); + List childFilterList = expectedTransform.getFilter().getChildFilterList(); + + assertTrue( + childFilterList + .get(0) + .getChildFilterList() + .contains(createContainsKeyFilter(spanTags1.getAttributeExpression()))); + assertTrue( + childFilterList + .get(1) + .getChildFilterList() + .contains(createContainsKeyFilter(spanTags2.getAttributeExpression()))); + } + + @Test + void transQueryWithComplexAttributeExpression_HierarchicalFilter() { + Expression spanTags1 = createComplexAttributeExpression("Span.tags", "FLAGS").build(); + Expression spanTags2 = createComplexAttributeExpression("Span.tags", "span.kind").build(); + + Filter filter1 = + Filter.newBuilder() + .setLhs(spanTags1) + .setOperator(Operator.EQ) + .setRhs(createStringLiteralValueExpression("0")) + .build(); + Filter filter2 = + Filter.newBuilder() + .setLhs(spanTags2) + .setOperator(Operator.EQ) + .setRhs(createStringLiteralValueExpression("server")) + .build(); + Filter filter = + createCompositeFilter( + Operator.AND, + createEqualsFilter("other-attribute", "otherValue"), + createCompositeFilter(Operator.AND, filter1, filter2).build()) + .build(); + + QueryRequest originalRequest = QueryRequest.newBuilder().setFilter(filter).build(); + + QueryRequest expectedTransform = + this.transformation.transform(originalRequest, mockTransformationContext).blockingGet(); + List childFilterList = + expectedTransform.getFilter().getChildFilterList().get(1).getChildFilterList(); + + assertTrue( + childFilterList + .get(0) + .getChildFilterList() + .contains(createContainsKeyFilter(spanTags1.getAttributeExpression()))); + assertTrue( + childFilterList + .get(1) + .getChildFilterList() + .contains(createContainsKeyFilter(spanTags2.getAttributeExpression()))); + } + + @Test + void transQueryWithComplexAttributeExpression_OrderByAndFilter() { + Expression.Builder spanTag = createComplexAttributeExpression("Span.tags", "span.kind"); + + Filter filter = + Filter.newBuilder() + .setLhs(spanTag) + .setOperator(Operator.EQ) + .setRhs(createStringLiteralValueExpression("server")) + .build(); + Filter containsKeyFilter = createContainsKeyFilter("Span.tags", "span.kind"); + + QueryRequest originalRequest = + QueryRequest.newBuilder() + .setFilter(filter) + .addOrderBy(createOrderByExpression(spanTag, SortOrder.ASC)) + .build(); + + QueryRequest expectedTransform = + QueryRequest.newBuilder() + .setFilter( + createCompositeFilter( + Operator.AND, + createCompositeFilter(Operator.AND, filter, containsKeyFilter).build(), + containsKeyFilter)) + .addOrderBy(createOrderByExpression(spanTag, SortOrder.ASC)) + .build(); + + assertEquals( + expectedTransform, + this.transformation.transform(originalRequest, mockTransformationContext).blockingGet()); + } + + @Test + void transQueryWithComplexAttributeExpression_SingleSelection() { + Expression.Builder spanTag = createComplexAttributeExpression("Span.tags", "span.kind"); + + QueryRequest originalRequest = QueryRequest.newBuilder().addSelection(spanTag).build(); + + QueryRequest expectedTransform = + QueryRequest.newBuilder() + .addSelection(spanTag) + .setFilter(createContainsKeyFilter("Span.tags", "span.kind")) + .build(); + + assertEquals( + expectedTransform, + this.transformation.transform(originalRequest, mockTransformationContext).blockingGet()); + } + + @Test + void transQueryWithComplexAttributeExpression_SingleOrderBy() { + Expression.Builder spanTag = createComplexAttributeExpression("Span.tags", "span.kind"); + + QueryRequest originalRequest = + QueryRequest.newBuilder() + .addOrderBy(createOrderByExpression(spanTag, SortOrder.ASC)) + .build(); + + QueryRequest expectedTransform = + QueryRequest.newBuilder() + .setFilter(createContainsKeyFilter("Span.tags", "span.kind")) + .addOrderBy(createOrderByExpression(spanTag, SortOrder.ASC)) + .build(); + + assertEquals( + expectedTransform, + this.transformation.transform(originalRequest, mockTransformationContext).blockingGet()); + } + + @Test + void transQueryWithComplexAttributeExpression_MultipleOrderBy() { + Expression.Builder spanTag1 = createComplexAttributeExpression("Span.tags", "span.kind"); + Expression.Builder spanTag2 = createComplexAttributeExpression("Span.tags", "FLAGS"); + + QueryRequest originalRequest = + QueryRequest.newBuilder() + .addOrderBy(createOrderByExpression(spanTag1, SortOrder.ASC)) + .addOrderBy(createOrderByExpression(spanTag2, SortOrder.ASC)) + .build(); + + QueryRequest expectedTransform = + QueryRequest.newBuilder() + .setFilter( + createCompositeFilter( + Operator.AND, + createContainsKeyFilter("Span.tags", "span.kind"), + createContainsKeyFilter("Span.tags", "FLAGS"))) + .addOrderBy(createOrderByExpression(spanTag1, SortOrder.ASC)) + .addOrderBy(createOrderByExpression(spanTag2, SortOrder.ASC)) + .build(); + + assertEquals( + expectedTransform, + this.transformation.transform(originalRequest, mockTransformationContext).blockingGet()); + } +} diff --git a/query-service-impl/src/test/java/org/hypertrace/core/query/service/projection/ProjectionTransformationTest.java b/query-service-impl/src/test/java/org/hypertrace/core/query/service/projection/ProjectionTransformationTest.java index 4ded70c5..e381ea07 100644 --- a/query-service-impl/src/test/java/org/hypertrace/core/query/service/projection/ProjectionTransformationTest.java +++ b/query-service-impl/src/test/java/org/hypertrace/core/query/service/projection/ProjectionTransformationTest.java @@ -10,23 +10,18 @@ import static org.hypertrace.core.query.service.QueryFunctionConstants.QUERY_FUNCTION_CONDITIONAL; import static org.hypertrace.core.query.service.QueryFunctionConstants.QUERY_FUNCTION_HASH; import static org.hypertrace.core.query.service.QueryFunctionConstants.QUERY_FUNCTION_STRINGEQUALS; -import static org.hypertrace.core.query.service.QueryRequestBuilderUtils.createAliasedColumnExpression; +import static org.hypertrace.core.query.service.QueryRequestBuilderUtils.createAliasedAttributeExpression; import static org.hypertrace.core.query.service.QueryRequestBuilderUtils.createAliasedFunctionExpression; -import static org.hypertrace.core.query.service.QueryRequestBuilderUtils.createColumnExpression; -import static org.hypertrace.core.query.service.QueryRequestBuilderUtils.createComplexAttributeExpression; import static org.hypertrace.core.query.service.QueryRequestBuilderUtils.createCompositeFilter; -import static org.hypertrace.core.query.service.QueryRequestBuilderUtils.createEqualsFilter; import static org.hypertrace.core.query.service.QueryRequestBuilderUtils.createFilter; import static org.hypertrace.core.query.service.QueryRequestBuilderUtils.createFunctionExpression; -import static org.hypertrace.core.query.service.QueryRequestBuilderUtils.createInFilter; import static org.hypertrace.core.query.service.QueryRequestBuilderUtils.createOrderByExpression; import static org.hypertrace.core.query.service.QueryRequestBuilderUtils.createStringArrayLiteralValueExpression; -import static org.hypertrace.core.query.service.QueryRequestUtil.createContainsKeyFilter; import static org.hypertrace.core.query.service.QueryRequestUtil.createNullNumberLiteralExpression; import static org.hypertrace.core.query.service.QueryRequestUtil.createNullStringLiteralExpression; +import static org.hypertrace.core.query.service.QueryRequestUtil.createSimpleAttributeExpression; import static org.hypertrace.core.query.service.QueryRequestUtil.createStringLiteralValueExpression; import static org.junit.jupiter.api.Assertions.assertEquals; -import static org.junit.jupiter.api.Assertions.assertTrue; import static org.mockito.Mockito.when; import io.reactivex.rxjava3.core.Single; @@ -42,8 +37,6 @@ import org.hypertrace.core.attribute.service.v1.ProjectionExpression; import org.hypertrace.core.attribute.service.v1.ProjectionOperator; import org.hypertrace.core.query.service.QueryTransformation.QueryTransformationContext; -import org.hypertrace.core.query.service.api.Expression; -import org.hypertrace.core.query.service.api.Filter; import org.hypertrace.core.query.service.api.Operator; import org.hypertrace.core.query.service.api.QueryRequest; import org.hypertrace.core.query.service.api.SortOrder; @@ -78,256 +71,18 @@ void beforeEach() { new ProjectionTransformation(this.mockAttributeClient, new AttributeProjectionRegistry()); } - @Test - void transQueryWithComplexAttributeExpression_SingleFilter() { - this.mockAttribute("server", AttributeMetadata.getDefaultInstance()); - this.mockAttribute("Span.tags", AttributeMetadata.getDefaultInstance()); - - Expression spanTags = createComplexAttributeExpression("Span.tags", "span.kind").build(); - Filter filter = - Filter.newBuilder() - .setLhs(spanTags) - .setOperator(Operator.EQ) - .setRhs(createColumnExpression("server")) - .build(); - QueryRequest originalRequest = QueryRequest.newBuilder().setFilter(filter).build(); - - QueryRequest expectedTransform = - QueryRequest.newBuilder() - .setFilter( - Filter.newBuilder() - .setOperator(Operator.AND) - .addAllChildFilter( - List.of(filter, createContainsKeyFilter(spanTags.getAttributeExpression()))) - .build()) - .build(); - - assertEquals( - expectedTransform, - this.projectionTransformation - .transform(originalRequest, mockTransformationContext) - .blockingGet()); - } - - @Test - void transQueryWithComplexAttributeExpression_MultipleFilter() { - this.mockAttribute("server", AttributeMetadata.getDefaultInstance()); - this.mockAttribute("0", AttributeMetadata.getDefaultInstance()); - this.mockAttribute("Span.tags", AttributeMetadata.getDefaultInstance()); - - Expression spanTags1 = createComplexAttributeExpression("Span.tags", "FLAGS").build(); - Expression spanTags2 = createComplexAttributeExpression("Span.tags", "span.kind").build(); - - Filter childFilter1 = - Filter.newBuilder() - .setLhs(spanTags1) - .setOperator(Operator.EQ) - .setRhs(createColumnExpression("0")) - .build(); - Filter childFilter2 = - Filter.newBuilder() - .setLhs(spanTags2) - .setOperator(Operator.EQ) - .setRhs(createColumnExpression("server")) - .build(); - - Filter.Builder filter = createCompositeFilter(Operator.AND, childFilter1, childFilter2); - QueryRequest originalRequest = QueryRequest.newBuilder().setFilter(filter).build(); - - QueryRequest expectedTransform = - this.projectionTransformation - .transform(originalRequest, mockTransformationContext) - .blockingGet(); - List childFilterList = expectedTransform.getFilter().getChildFilterList(); - - assertTrue( - childFilterList - .get(0) - .getChildFilterList() - .contains(createContainsKeyFilter(spanTags1.getAttributeExpression()))); - assertTrue( - childFilterList - .get(1) - .getChildFilterList() - .contains(createContainsKeyFilter(spanTags2.getAttributeExpression()))); - } - - @Test - void transQueryWithComplexAttributeExpression_HierarchicalFilter() { - this.mockAttribute("server", AttributeMetadata.getDefaultInstance()); - this.mockAttribute("0", AttributeMetadata.getDefaultInstance()); - this.mockAttribute(SIMPLE_ATTRIBUTE_ID, AttributeMetadata.getDefaultInstance()); - this.mockAttribute("Span.tags", AttributeMetadata.getDefaultInstance()); - - Expression spanTags1 = createComplexAttributeExpression("Span.tags", "FLAGS").build(); - Expression spanTags2 = createComplexAttributeExpression("Span.tags", "span.kind").build(); - - Filter filter1 = - Filter.newBuilder() - .setLhs(spanTags1) - .setOperator(Operator.EQ) - .setRhs(createColumnExpression("0")) - .build(); - Filter filter2 = - Filter.newBuilder() - .setLhs(spanTags2) - .setOperator(Operator.EQ) - .setRhs(createColumnExpression("server")) - .build(); - Filter filter = - createCompositeFilter( - Operator.AND, - createEqualsFilter(SIMPLE_ATTRIBUTE_ID, "otherValue"), - createCompositeFilter(Operator.AND, filter1, filter2).build()) - .build(); - - QueryRequest originalRequest = QueryRequest.newBuilder().setFilter(filter).build(); - - QueryRequest expectedTransform = - this.projectionTransformation - .transform(originalRequest, mockTransformationContext) - .blockingGet(); - List childFilterList = - expectedTransform.getFilter().getChildFilterList().get(1).getChildFilterList(); - - assertTrue( - childFilterList - .get(0) - .getChildFilterList() - .contains(createContainsKeyFilter(spanTags1.getAttributeExpression()))); - assertTrue( - childFilterList - .get(1) - .getChildFilterList() - .contains(createContainsKeyFilter(spanTags2.getAttributeExpression()))); - } - - @Test - void transQueryWithComplexAttributeExpression_OrderByAndFilter() { - this.mockAttribute("server", AttributeMetadata.getDefaultInstance()); - this.mockAttribute("Span.tags", AttributeMetadata.getDefaultInstance()); - - Expression.Builder spanTag = createComplexAttributeExpression("Span.tags", "span.kind"); - - Filter filter = - Filter.newBuilder() - .setLhs(spanTag) - .setOperator(Operator.EQ) - .setRhs(createColumnExpression("server")) - .build(); - Filter containsKeyFilter = createContainsKeyFilter("Span.tags", "span.kind"); - - QueryRequest originalRequest = - QueryRequest.newBuilder() - .setFilter(filter) - .addOrderBy(createOrderByExpression(spanTag, SortOrder.ASC)) - .build(); - - QueryRequest expectedTransform = - QueryRequest.newBuilder() - .setFilter( - createCompositeFilter( - Operator.AND, - createCompositeFilter(Operator.AND, filter, containsKeyFilter).build(), - containsKeyFilter)) - .addOrderBy(createOrderByExpression(spanTag, SortOrder.ASC)) - .build(); - - assertEquals( - expectedTransform, - this.projectionTransformation - .transform(originalRequest, mockTransformationContext) - .blockingGet()); - } - - @Test - void transQueryWithComplexAttributeExpression_SingleSelection() { - this.mockAttribute("Span.tags", AttributeMetadata.getDefaultInstance()); - - Expression.Builder spanTag = createComplexAttributeExpression("Span.tags", "span.kind"); - - QueryRequest originalRequest = QueryRequest.newBuilder().addSelection(spanTag).build(); - - QueryRequest expectedTransform = - QueryRequest.newBuilder() - .addSelection(spanTag) - .setFilter(createContainsKeyFilter("Span.tags", "span.kind")) - .build(); - - assertEquals( - expectedTransform, - this.projectionTransformation - .transform(originalRequest, mockTransformationContext) - .blockingGet()); - } - - @Test - void transQueryWithComplexAttributeExpression_SingleOrderBy() { - this.mockAttribute("Span.tags", AttributeMetadata.getDefaultInstance()); - - Expression.Builder spanTag = createComplexAttributeExpression("Span.tags", "span.kind"); - - QueryRequest originalRequest = - QueryRequest.newBuilder() - .addOrderBy(createOrderByExpression(spanTag, SortOrder.ASC)) - .build(); - - QueryRequest expectedTransform = - QueryRequest.newBuilder() - .setFilter(createContainsKeyFilter("Span.tags", "span.kind")) - .addOrderBy(createOrderByExpression(spanTag, SortOrder.ASC)) - .build(); - - assertEquals( - expectedTransform, - this.projectionTransformation - .transform(originalRequest, mockTransformationContext) - .blockingGet()); - } - - @Test - void transQueryWithComplexAttributeExpression_MultipleOrderBy() { - this.mockAttribute("Span.tags", AttributeMetadata.getDefaultInstance()); - - Expression.Builder spanTag1 = createComplexAttributeExpression("Span.tags", "span.kind"); - Expression.Builder spanTag2 = createComplexAttributeExpression("Span.tags", "FLAGS"); - - QueryRequest originalRequest = - QueryRequest.newBuilder() - .addOrderBy(createOrderByExpression(spanTag1, SortOrder.ASC)) - .addOrderBy(createOrderByExpression(spanTag2, SortOrder.ASC)) - .build(); - - QueryRequest expectedTransform = - QueryRequest.newBuilder() - .setFilter( - createCompositeFilter( - Operator.AND, - createContainsKeyFilter("Span.tags", "span.kind"), - createContainsKeyFilter("Span.tags", "FLAGS"))) - .addOrderBy(createOrderByExpression(spanTag1, SortOrder.ASC)) - .addOrderBy(createOrderByExpression(spanTag2, SortOrder.ASC)) - .build(); - - assertEquals( - expectedTransform, - this.projectionTransformation - .transform(originalRequest, mockTransformationContext) - .blockingGet()); - } - @Test void transformsBasicAliasProjection() { this.mockAttribute(PROJECTED_ATTRIBUTE_ID, this.attributeMetadata); this.mockAttribute(SIMPLE_ATTRIBUTE_ID, AttributeMetadata.getDefaultInstance()); QueryRequest originalRequest = QueryRequest.newBuilder() - .addSelection(createColumnExpression(PROJECTED_ATTRIBUTE_ID)) + .addSelection(createSimpleAttributeExpression(PROJECTED_ATTRIBUTE_ID)) .build(); QueryRequest expectedTransform = QueryRequest.newBuilder() .addSelection( - createAliasedColumnExpression(SIMPLE_ATTRIBUTE_ID, PROJECTED_ATTRIBUTE_ID)) + createAliasedAttributeExpression(SIMPLE_ATTRIBUTE_ID, PROJECTED_ATTRIBUTE_ID)) .build(); assertEquals( @@ -357,7 +112,7 @@ PROJECTION_OPERATOR_HASH, attributeIdProjection(SIMPLE_ATTRIBUTE_ID))), QueryRequest originalRequest = QueryRequest.newBuilder() - .addSelection(createColumnExpression(PROJECTED_ATTRIBUTE_ID)) + .addSelection(createSimpleAttributeExpression(PROJECTED_ATTRIBUTE_ID)) .build(); QueryRequest expectedTransform = @@ -367,7 +122,8 @@ PROJECTION_OPERATOR_HASH, attributeIdProjection(SIMPLE_ATTRIBUTE_ID))), QUERY_FUNCTION_CONCAT, PROJECTED_ATTRIBUTE_ID, createFunctionExpression( - QUERY_FUNCTION_HASH, createColumnExpression(SIMPLE_ATTRIBUTE_ID).build()), + QUERY_FUNCTION_HASH, + createSimpleAttributeExpression(SIMPLE_ATTRIBUTE_ID).build()), createStringLiteralValueExpression("projectionLiteral"))) .build(); @@ -405,7 +161,7 @@ PROJECTION_OPERATOR_HASH, attributeIdProjection(SIMPLE_ATTRIBUTE_ID))), QueryRequest originalRequest = QueryRequest.newBuilder() - .addSelection(createColumnExpression(PROJECTED_ATTRIBUTE_ID)) + .addSelection(createSimpleAttributeExpression(PROJECTED_ATTRIBUTE_ID)) .build(); QueryRequest expectedTransform = @@ -416,10 +172,11 @@ PROJECTION_OPERATOR_HASH, attributeIdProjection(SIMPLE_ATTRIBUTE_ID))), PROJECTED_ATTRIBUTE_ID, createFunctionExpression( QUERY_FUNCTION_STRINGEQUALS, - createColumnExpression(SIMPLE_ATTRIBUTE_ID).build(), + createSimpleAttributeExpression(SIMPLE_ATTRIBUTE_ID).build(), createStringLiteralValueExpression("foo")), createFunctionExpression( - QUERY_FUNCTION_HASH, createColumnExpression(SIMPLE_ATTRIBUTE_ID).build()), + QUERY_FUNCTION_HASH, + createSimpleAttributeExpression(SIMPLE_ATTRIBUTE_ID).build()), createStringLiteralValueExpression("projectionLiteral"))) .build(); @@ -440,7 +197,7 @@ void transformsAggregations() { createAliasedFunctionExpression( QUERY_FUNCTION_AVG, "myAlias", - createColumnExpression(PROJECTED_ATTRIBUTE_ID).build())) + createSimpleAttributeExpression(PROJECTED_ATTRIBUTE_ID).build())) .build(); QueryRequest expectedTransform = QueryRequest.newBuilder() @@ -448,7 +205,7 @@ void transformsAggregations() { createAliasedFunctionExpression( QUERY_FUNCTION_AVG, "myAlias", - createAliasedColumnExpression(SIMPLE_ATTRIBUTE_ID, PROJECTED_ATTRIBUTE_ID))) + createAliasedAttributeExpression(SIMPLE_ATTRIBUTE_ID, PROJECTED_ATTRIBUTE_ID))) .build(); assertEquals( @@ -464,18 +221,18 @@ void transformsOrderBys() { this.mockAttribute(SIMPLE_ATTRIBUTE_ID, AttributeMetadata.getDefaultInstance()); QueryRequest originalRequest = QueryRequest.newBuilder() - .addSelection(createColumnExpression(PROJECTED_ATTRIBUTE_ID)) + .addSelection(createSimpleAttributeExpression(PROJECTED_ATTRIBUTE_ID)) .addOrderBy( createOrderByExpression( - createColumnExpression(PROJECTED_ATTRIBUTE_ID), SortOrder.DESC)) + createSimpleAttributeExpression(PROJECTED_ATTRIBUTE_ID), SortOrder.DESC)) .build(); QueryRequest expectedTransform = QueryRequest.newBuilder() .addSelection( - createAliasedColumnExpression(SIMPLE_ATTRIBUTE_ID, PROJECTED_ATTRIBUTE_ID)) + createAliasedAttributeExpression(SIMPLE_ATTRIBUTE_ID, PROJECTED_ATTRIBUTE_ID)) .addOrderBy( createOrderByExpression( - createAliasedColumnExpression(SIMPLE_ATTRIBUTE_ID, PROJECTED_ATTRIBUTE_ID) + createAliasedAttributeExpression(SIMPLE_ATTRIBUTE_ID, PROJECTED_ATTRIBUTE_ID) .toBuilder(), SortOrder.DESC)) .build(); @@ -493,26 +250,36 @@ void transformsNestedFilters() { this.mockAttribute(SIMPLE_ATTRIBUTE_ID, AttributeMetadata.getDefaultInstance()); QueryRequest originalRequest = QueryRequest.newBuilder() - .addSelection(createColumnExpression(PROJECTED_ATTRIBUTE_ID)) + .addSelection(createSimpleAttributeExpression(PROJECTED_ATTRIBUTE_ID)) .setFilter( createCompositeFilter( Operator.OR, - createInFilter(PROJECTED_ATTRIBUTE_ID, List.of("foo", "bar")), - createEqualsFilter(SIMPLE_ATTRIBUTE_ID, "otherValue"))) + createFilter( + createSimpleAttributeExpression(PROJECTED_ATTRIBUTE_ID).build(), + Operator.IN, + createStringArrayLiteralValueExpression(List.of("foo", "bar"))), + createFilter( + createSimpleAttributeExpression(SIMPLE_ATTRIBUTE_ID).build(), + Operator.EQ, + createStringLiteralValueExpression("otherValue")))) .build(); QueryRequest expectedTransform = QueryRequest.newBuilder() .addSelection( - createAliasedColumnExpression(SIMPLE_ATTRIBUTE_ID, PROJECTED_ATTRIBUTE_ID)) + createAliasedAttributeExpression(SIMPLE_ATTRIBUTE_ID, PROJECTED_ATTRIBUTE_ID)) .setFilter( createCompositeFilter( Operator.OR, createFilter( - createAliasedColumnExpression(SIMPLE_ATTRIBUTE_ID, PROJECTED_ATTRIBUTE_ID), + createAliasedAttributeExpression( + SIMPLE_ATTRIBUTE_ID, PROJECTED_ATTRIBUTE_ID), Operator.IN, createStringArrayLiteralValueExpression(List.of("foo", "bar"))), - createEqualsFilter(SIMPLE_ATTRIBUTE_ID, "otherValue"))) + createFilter( + createSimpleAttributeExpression(SIMPLE_ATTRIBUTE_ID).build(), + Operator.EQ, + createStringLiteralValueExpression("otherValue")))) .build(); assertEquals( @@ -530,16 +297,18 @@ void transformsGroupBys() { QueryRequest.newBuilder() .addAggregation( createFunctionExpression( - QUERY_FUNCTION_AVG, createColumnExpression(PROJECTED_ATTRIBUTE_ID).build())) - .addGroupBy(createColumnExpression(PROJECTED_ATTRIBUTE_ID)) + QUERY_FUNCTION_AVG, + createSimpleAttributeExpression(PROJECTED_ATTRIBUTE_ID).build())) + .addGroupBy(createSimpleAttributeExpression(PROJECTED_ATTRIBUTE_ID)) .build(); QueryRequest expectedTransform = QueryRequest.newBuilder() .addAggregation( createFunctionExpression( QUERY_FUNCTION_AVG, - createAliasedColumnExpression(SIMPLE_ATTRIBUTE_ID, PROJECTED_ATTRIBUTE_ID))) - .addGroupBy(createAliasedColumnExpression(SIMPLE_ATTRIBUTE_ID, PROJECTED_ATTRIBUTE_ID)) + createAliasedAttributeExpression(SIMPLE_ATTRIBUTE_ID, PROJECTED_ATTRIBUTE_ID))) + .addGroupBy( + createAliasedAttributeExpression(SIMPLE_ATTRIBUTE_ID, PROJECTED_ATTRIBUTE_ID)) .build(); assertEquals( @@ -562,16 +331,16 @@ void passesThroughExpressionsInOrder() { QueryRequest originalRequest = QueryRequest.newBuilder() - .addSelection(createColumnExpression("slow")) - .addSelection(createColumnExpression(SIMPLE_ATTRIBUTE_ID)) - .addSelection(createColumnExpression(PROJECTED_ATTRIBUTE_ID)) + .addSelection(createSimpleAttributeExpression("slow")) + .addSelection(createSimpleAttributeExpression(SIMPLE_ATTRIBUTE_ID)) + .addSelection(createSimpleAttributeExpression(PROJECTED_ATTRIBUTE_ID)) .build(); QueryRequest expected = QueryRequest.newBuilder() - .addSelection(createColumnExpression("slow")) - .addSelection(createColumnExpression(SIMPLE_ATTRIBUTE_ID)) + .addSelection(createSimpleAttributeExpression("slow")) + .addSelection(createSimpleAttributeExpression(SIMPLE_ATTRIBUTE_ID)) .addSelection( - createAliasedColumnExpression(SIMPLE_ATTRIBUTE_ID, PROJECTED_ATTRIBUTE_ID)) + createAliasedAttributeExpression(SIMPLE_ATTRIBUTE_ID, PROJECTED_ATTRIBUTE_ID)) .build(); assertEquals( expected, @@ -593,17 +362,19 @@ void passesThroughOrderBysInOrder() { QueryRequest originalRequest = QueryRequest.newBuilder() - .addOrderBy(createOrderByExpression(createColumnExpression("slow"), SortOrder.ASC)) + .addOrderBy( + createOrderByExpression(createSimpleAttributeExpression("slow"), SortOrder.ASC)) .addOrderBy( createOrderByExpression( - createColumnExpression(PROJECTED_ATTRIBUTE_ID), SortOrder.DESC)) + createSimpleAttributeExpression(PROJECTED_ATTRIBUTE_ID), SortOrder.DESC)) .build(); QueryRequest expectedTransform = QueryRequest.newBuilder() - .addOrderBy(createOrderByExpression(createColumnExpression("slow"), SortOrder.ASC)) + .addOrderBy( + createOrderByExpression(createSimpleAttributeExpression("slow"), SortOrder.ASC)) .addOrderBy( createOrderByExpression( - createAliasedColumnExpression(SIMPLE_ATTRIBUTE_ID, PROJECTED_ATTRIBUTE_ID) + createAliasedAttributeExpression(SIMPLE_ATTRIBUTE_ID, PROJECTED_ATTRIBUTE_ID) .toBuilder(), SortOrder.DESC)) .build(); @@ -628,14 +399,14 @@ void transformNullDefinedAttributes() { QueryRequest originalRequest = QueryRequest.newBuilder() - .addSelection(createColumnExpression(PROJECTED_ATTRIBUTE_ID)) - .addSelection(createColumnExpression(SIMPLE_ATTRIBUTE_ID)) + .addSelection(createSimpleAttributeExpression(PROJECTED_ATTRIBUTE_ID)) + .addSelection(createSimpleAttributeExpression(SIMPLE_ATTRIBUTE_ID)) .build(); QueryRequest expectedTransform = QueryRequest.newBuilder() .addSelection(createNullStringLiteralExpression()) - .addSelection(createColumnExpression(SIMPLE_ATTRIBUTE_ID)) + .addSelection(createSimpleAttributeExpression(SIMPLE_ATTRIBUTE_ID)) .build(); assertEquals( @@ -656,7 +427,7 @@ void transformNullDefinedAttributes() { expectedTransform = QueryRequest.newBuilder() .addSelection(createNullNumberLiteralExpression()) - .addSelection(createColumnExpression(SIMPLE_ATTRIBUTE_ID)) + .addSelection(createSimpleAttributeExpression(SIMPLE_ATTRIBUTE_ID)) .build(); assertEquals( @@ -687,7 +458,7 @@ void transformsNullProjectionArguments() { QueryRequest originalRequest = QueryRequest.newBuilder() - .addSelection(createColumnExpression(PROJECTED_ATTRIBUTE_ID)) + .addSelection(createSimpleAttributeExpression(PROJECTED_ATTRIBUTE_ID)) .build(); QueryRequest expectedTransform = @@ -697,7 +468,7 @@ void transformsNullProjectionArguments() { QUERY_FUNCTION_CONDITIONAL, PROJECTED_ATTRIBUTE_ID, createStringLiteralValueExpression("true"), - createColumnExpression(SIMPLE_ATTRIBUTE_ID).build(), + createSimpleAttributeExpression(SIMPLE_ATTRIBUTE_ID).build(), createNullStringLiteralExpression())) .build(); @@ -714,7 +485,7 @@ void doesNotTransformMaterializedDefinition() { PROJECTED_ATTRIBUTE_ID, this.attributeMetadata.toBuilder().setMaterialized(true).build()); QueryRequest originalRequest = QueryRequest.newBuilder() - .addSelection(createColumnExpression(PROJECTED_ATTRIBUTE_ID)) + .addSelection(createSimpleAttributeExpression(PROJECTED_ATTRIBUTE_ID)) .build(); assertEquals( From cb7623224288d2e996e4bb81016c42ecebf4fd92 Mon Sep 17 00:00:00 2001 From: Aaron Steinfeld Date: Thu, 23 Dec 2021 13:11:51 -0500 Subject: [PATCH 19/23] style: fix nit --- .../core/query/service/pinot/PinotBasedRequestHandler.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/query-service-impl/src/main/java/org/hypertrace/core/query/service/pinot/PinotBasedRequestHandler.java b/query-service-impl/src/main/java/org/hypertrace/core/query/service/pinot/PinotBasedRequestHandler.java index 12d37ed4..0d1084ad 100644 --- a/query-service-impl/src/main/java/org/hypertrace/core/query/service/pinot/PinotBasedRequestHandler.java +++ b/query-service-impl/src/main/java/org/hypertrace/core/query/service/pinot/PinotBasedRequestHandler.java @@ -279,7 +279,7 @@ private boolean doesSingleViewFilterMatchLeafQueryFilter( } ViewColumnFilter viewColumnFilter = - viewFilterMap.get(getLogicalColumnName(queryFilter.getLhs()).orElse(null)); + getLogicalColumnName(queryFilter.getLhs()).map(viewFilterMap::get).orElse(null); if (viewColumnFilter == null) { return false; } From 52bc346c9f53c5302ee11dd65c2bc9cc4223ae94 Mon Sep 17 00:00:00 2001 From: Aaron Steinfeld <45047841+aaron-steinfeld@users.noreply.github.com> Date: Thu, 23 Dec 2021 20:21:30 -0800 Subject: [PATCH 20/23] Apply suggestions from code review Co-authored-by: sarthak --- ...uteExpressionSubpathExistsFilteringTransformation.java | 8 ++++---- ...ttributeExpressionNormalizationTransformationTest.java | 2 +- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/query-service-impl/src/main/java/org/hypertrace/core/query/service/attribubteexpression/AttributeExpressionSubpathExistsFilteringTransformation.java b/query-service-impl/src/main/java/org/hypertrace/core/query/service/attribubteexpression/AttributeExpressionSubpathExistsFilteringTransformation.java index eccce03d..371ccf70 100644 --- a/query-service-impl/src/main/java/org/hypertrace/core/query/service/attribubteexpression/AttributeExpressionSubpathExistsFilteringTransformation.java +++ b/query-service-impl/src/main/java/org/hypertrace/core/query/service/attribubteexpression/AttributeExpressionSubpathExistsFilteringTransformation.java @@ -47,10 +47,10 @@ protected QueryRequest rebuildRequest( } /* - * We need the CONTAINS_KEY filter in all filters and order bys dealing with complex - * attribute expressions as Pinot gives error if particular key is absent. Rest all work fine. - * To handle order bys, we add the corresponding filter at the top and 'AND' it with the main filter. - * To handle filter, we modify each filter (say filter1) as : "CONTAINS_KEY AND filter1". + * We need the CONTAINS_KEY filter in all filters, selections, and order bys dealing with complex + * attribute expressions as Pinot gives an error if a particular key is absent. Rest all work fine. + * To handle order bys and selections, we add the corresponding filter at the top and 'AND' it with the main filter. + * To handle filter, we modify each filter (say filter1) as: "CONTAINS_KEY AND filter1". */ private Optional rebuildFilterForComplexAttributeExpression( Filter originalFilter, List orderBys, List selections) { diff --git a/query-service-impl/src/test/java/org/hypertrace/core/query/service/attribubteexpression/AttributeExpressionNormalizationTransformationTest.java b/query-service-impl/src/test/java/org/hypertrace/core/query/service/attribubteexpression/AttributeExpressionNormalizationTransformationTest.java index 5ac7975b..00d714f6 100644 --- a/query-service-impl/src/test/java/org/hypertrace/core/query/service/attribubteexpression/AttributeExpressionNormalizationTransformationTest.java +++ b/query-service-impl/src/test/java/org/hypertrace/core/query/service/attribubteexpression/AttributeExpressionNormalizationTransformationTest.java @@ -28,7 +28,7 @@ class AttributeExpressionNormalizationTransformationTest { new AttributeExpressionNormalizationTransformation(); @Test - void columnInAllPositions() { + void transformColumnInAllPositions() { QueryRequest originalRequest = QueryRequest.newBuilder() .addSelection(createAliasedColumnExpression("select-col", "select-alias")) From d13c3d7052b2f1ce0730848bea5c79b8b8b94f90 Mon Sep 17 00:00:00 2001 From: Sarthak Singhal Date: Mon, 27 Dec 2021 11:33:59 +0530 Subject: [PATCH 21/23] added unit test for contains_key_value op --- .../query/service/pinot/MigrationTest.java | 31 +++++++++++++++++++ 1 file changed, 31 insertions(+) diff --git a/query-service-impl/src/test/java/org/hypertrace/core/query/service/pinot/MigrationTest.java b/query-service-impl/src/test/java/org/hypertrace/core/query/service/pinot/MigrationTest.java index 8d2ecfb9..3a0ade2c 100644 --- a/query-service-impl/src/test/java/org/hypertrace/core/query/service/pinot/MigrationTest.java +++ b/query-service-impl/src/test/java/org/hypertrace/core/query/service/pinot/MigrationTest.java @@ -9,6 +9,7 @@ import static org.hypertrace.core.query.service.QueryRequestBuilderUtils.createFunctionExpression; import static org.hypertrace.core.query.service.QueryRequestBuilderUtils.createNullStringLiteralValueExpression; import static org.hypertrace.core.query.service.QueryRequestBuilderUtils.createOrderByExpression; +import static org.hypertrace.core.query.service.QueryRequestBuilderUtils.createStringArrayLiteralValueExpression; import static org.hypertrace.core.query.service.QueryRequestBuilderUtils.createTimeFilterWithSimpleAttribute; import static org.hypertrace.core.query.service.QueryRequestUtil.createContainsKeyFilter; import static org.hypertrace.core.query.service.QueryRequestUtil.createSimpleAttributeExpression; @@ -20,6 +21,7 @@ import com.typesafe.config.Config; import com.typesafe.config.ConfigFactory; import java.util.LinkedHashSet; +import java.util.List; import java.util.Map.Entry; import org.apache.pinot.client.Connection; import org.apache.pinot.client.Request; @@ -226,6 +228,35 @@ public void testQueryWithNotContainsKeyOperator() { executionContext); } + @Test + public void testQueryWithContainsKeyValueOperator() { + Builder builder = QueryRequest.newBuilder(); + Expression spanTag = createSimpleAttributeExpression("Span.tags").build(); + builder.addSelection(spanTag); + builder.setFilter( + Filter.newBuilder() + .setOperator(Operator.CONTAINS_KEYVALUE) + .setLhs(spanTag) + .setRhs(createStringArrayLiteralValueExpression(List.of("Flags", "0"))) + .build()); + + ViewDefinition viewDefinition = getDefaultViewDefinition(); + defaultMockingForExecutionContext(); + + assertPQLQuery( + builder.build(), + "SELECT tags__keys, tags__values FROM SpanEventView " + + "WHERE " + + viewDefinition.getTenantIdColumn() + + " = '" + + TENANT_ID + + "' " + + "AND tags__keys = 'flags' and tags__values = '0' " + + "AND mapvalue(tags__keys,'flags',tags__values) = '0'", + viewDefinition, + executionContext); + } + @Test public void testQueryWithOrderByWithPagination() { QueryRequest orderByQueryRequest = buildOrderByQuery(); From a52fa5e414ad9bea3b7675f34793234bf215f4ef Mon Sep 17 00:00:00 2001 From: Aaron Steinfeld Date: Mon, 27 Dec 2021 15:17:04 -0500 Subject: [PATCH 22/23] chore: update dependencies --- query-service-api/build.gradle.kts | 7 ++++--- query-service-client/build.gradle.kts | 2 +- query-service-impl/build.gradle.kts | 9 +++------ query-service/build.gradle.kts | 19 +++++-------------- 4 files changed, 13 insertions(+), 24 deletions(-) diff --git a/query-service-api/build.gradle.kts b/query-service-api/build.gradle.kts index be6fd460..ffa987de 100644 --- a/query-service-api/build.gradle.kts +++ b/query-service-api/build.gradle.kts @@ -23,7 +23,7 @@ protobuf { // the identifier, which can be referred to in the "plugins" // container of the "generateProtoTasks" closure. id("grpc_java") { - artifact = "io.grpc:protoc-gen-grpc-java:1.42.0" + artifact = "io.grpc:protoc-gen-grpc-java:1.43.1" } if (generateLocalGoGrpcFiles) { @@ -66,8 +66,9 @@ tasks.test { } dependencies { - api("io.grpc:grpc-protobuf:1.42.0") - api("io.grpc:grpc-stub:1.42.0") + api(platform("io.grpc:grpc-bom:1.43.1")) + api("io.grpc:grpc-protobuf") + api("io.grpc:grpc-stub") api("javax.annotation:javax.annotation-api:1.3.2") testImplementation("org.junit.jupiter:junit-jupiter:5.7.1") diff --git a/query-service-client/build.gradle.kts b/query-service-client/build.gradle.kts index b7d28c73..5f534012 100644 --- a/query-service-client/build.gradle.kts +++ b/query-service-client/build.gradle.kts @@ -7,7 +7,7 @@ plugins { dependencies { api(project(":query-service-api")) - implementation("org.hypertrace.core.grpcutils:grpc-client-utils:0.6.2") + implementation("org.hypertrace.core.grpcutils:grpc-client-utils:0.7.0") // Logging implementation("org.slf4j:slf4j-api:1.7.30") diff --git a/query-service-impl/build.gradle.kts b/query-service-impl/build.gradle.kts index 0d0c9769..59195777 100644 --- a/query-service-impl/build.gradle.kts +++ b/query-service-impl/build.gradle.kts @@ -10,9 +10,6 @@ tasks.test { dependencies { constraints { - implementation("com.fasterxml.jackson.core:jackson-databind:2.12.2") { - because("Multiple vulnerabilities") - } implementation("io.netty:netty:3.10.6.Final") { because("https://snyk.io/vuln/SNYK-JAVA-IONETTY-30430") } @@ -28,9 +25,9 @@ dependencies { } api(project(":query-service-api")) api("com.typesafe:config:1.4.1") - implementation("org.hypertrace.core.grpcutils:grpc-context-utils:0.6.2") - implementation("org.hypertrace.core.grpcutils:grpc-client-utils:0.6.2") - implementation("org.hypertrace.core.grpcutils:grpc-server-rx-utils:0.6.2") + implementation("org.hypertrace.core.grpcutils:grpc-context-utils:0.7.0") + implementation("org.hypertrace.core.grpcutils:grpc-client-utils:0.7.0") + implementation("org.hypertrace.core.grpcutils:grpc-server-rx-utils:0.7.0") implementation("org.hypertrace.core.attribute.service:attribute-service-api:0.12.3") implementation("org.hypertrace.core.attribute.service:attribute-projection-registry:0.12.3") implementation("org.hypertrace.core.attribute.service:caching-attribute-service-client:0.12.3") diff --git a/query-service/build.gradle.kts b/query-service/build.gradle.kts index edec2e29..78c0c4d8 100644 --- a/query-service/build.gradle.kts +++ b/query-service/build.gradle.kts @@ -10,29 +10,20 @@ plugins { dependencies { implementation(project(":query-service-impl")) - implementation("org.hypertrace.core.grpcutils:grpc-server-utils:0.6.2") + implementation("org.hypertrace.core.grpcutils:grpc-server-utils:0.7.0") implementation("org.hypertrace.core.serviceframework:platform-service-framework:0.1.28") implementation("org.slf4j:slf4j-api:1.7.30") implementation("com.typesafe:config:1.4.1") runtimeOnly("org.apache.logging.log4j:log4j-slf4j-impl:2.17.0") - runtimeOnly("io.grpc:grpc-netty:1.42.0") - constraints { - runtimeOnly("io.netty:netty-codec-http2:4.1.71.Final") { - because("https://snyk.io/vuln/SNYK-JAVA-IONETTY-1089809") - } - runtimeOnly("io.netty:netty-handler-proxy:4.1.71.Final") { - because("https://snyk.io/vuln/SNYK-JAVA-IONETTY-1089809") - } - } - + runtimeOnly("io.grpc:grpc-netty") integrationTestImplementation("com.google.protobuf:protobuf-java-util:3.17.3") integrationTestImplementation("org.junit.jupiter:junit-jupiter-api:5.7.1") integrationTestImplementation("org.junit.jupiter:junit-jupiter-params:5.7.1") integrationTestImplementation("org.junit.jupiter:junit-jupiter-engine:5.7.1") - integrationTestImplementation("org.testcontainers:testcontainers:1.15.2") - integrationTestImplementation("org.testcontainers:junit-jupiter:1.15.2") - integrationTestImplementation("org.testcontainers:kafka:1.15.2") + integrationTestImplementation("org.testcontainers:testcontainers:1.16.2") + integrationTestImplementation("org.testcontainers:junit-jupiter:1.16.2") + integrationTestImplementation("org.testcontainers:kafka:1.16.2") integrationTestImplementation("org.hypertrace.core.serviceframework:integrationtest-service-framework:0.1.28") integrationTestImplementation("com.github.stefanbirkner:system-lambda:1.2.0") From fa6bc5425b66eb7046725caae66588d46ba5ca0b Mon Sep 17 00:00:00 2001 From: Aaron Steinfeld Date: Mon, 27 Dec 2021 15:51:00 -0500 Subject: [PATCH 23/23] chore: fix integration tests --- query-service-client/build.gradle.kts | 2 +- query-service-impl/build.gradle.kts | 6 +++--- query-service/build.gradle.kts | 10 ++++------ .../query/service/htqueries/HTPinotQueriesTest.java | 3 +-- 4 files changed, 9 insertions(+), 12 deletions(-) diff --git a/query-service-client/build.gradle.kts b/query-service-client/build.gradle.kts index 5f534012..b7529cd9 100644 --- a/query-service-client/build.gradle.kts +++ b/query-service-client/build.gradle.kts @@ -10,7 +10,7 @@ dependencies { implementation("org.hypertrace.core.grpcutils:grpc-client-utils:0.7.0") // Logging - implementation("org.slf4j:slf4j-api:1.7.30") + implementation("org.slf4j:slf4j-api:1.7.32") // Config implementation("com.typesafe:config:1.4.1") } diff --git a/query-service-impl/build.gradle.kts b/query-service-impl/build.gradle.kts index 59195777..c1a0f1b3 100644 --- a/query-service-impl/build.gradle.kts +++ b/query-service-impl/build.gradle.kts @@ -31,15 +31,15 @@ dependencies { implementation("org.hypertrace.core.attribute.service:attribute-service-api:0.12.3") implementation("org.hypertrace.core.attribute.service:attribute-projection-registry:0.12.3") implementation("org.hypertrace.core.attribute.service:caching-attribute-service-client:0.12.3") - implementation("org.hypertrace.core.serviceframework:service-framework-spi:0.1.28") + implementation("org.hypertrace.core.serviceframework:service-framework-spi:0.1.33") implementation("com.google.inject:guice:5.0.1") implementation("org.apache.pinot:pinot-java-client:0.6.0") { // We want to use log4j2 impl so exclude the log4j binding of slf4j exclude("org.slf4j", "slf4j-log4j12") } - implementation("org.slf4j:slf4j-api:1.7.30") + implementation("org.slf4j:slf4j-api:1.7.32") implementation("commons-codec:commons-codec:1.15") - implementation("org.hypertrace.core.serviceframework:platform-metrics:0.1.28") + implementation("org.hypertrace.core.serviceframework:platform-metrics:0.1.33") implementation("com.google.protobuf:protobuf-java-util:3.15.6") implementation("com.google.guava:guava:30.1.1-jre") implementation("io.reactivex.rxjava3:rxjava:3.0.11") diff --git a/query-service/build.gradle.kts b/query-service/build.gradle.kts index 78c0c4d8..76e19ebd 100644 --- a/query-service/build.gradle.kts +++ b/query-service/build.gradle.kts @@ -11,20 +11,18 @@ plugins { dependencies { implementation(project(":query-service-impl")) implementation("org.hypertrace.core.grpcutils:grpc-server-utils:0.7.0") - implementation("org.hypertrace.core.serviceframework:platform-service-framework:0.1.28") - implementation("org.slf4j:slf4j-api:1.7.30") + implementation("org.hypertrace.core.serviceframework:platform-service-framework:0.1.33") + implementation("org.slf4j:slf4j-api:1.7.32") implementation("com.typesafe:config:1.4.1") runtimeOnly("org.apache.logging.log4j:log4j-slf4j-impl:2.17.0") runtimeOnly("io.grpc:grpc-netty") integrationTestImplementation("com.google.protobuf:protobuf-java-util:3.17.3") - integrationTestImplementation("org.junit.jupiter:junit-jupiter-api:5.7.1") - integrationTestImplementation("org.junit.jupiter:junit-jupiter-params:5.7.1") - integrationTestImplementation("org.junit.jupiter:junit-jupiter-engine:5.7.1") + integrationTestImplementation("org.junit.jupiter:junit-jupiter:5.7.1") integrationTestImplementation("org.testcontainers:testcontainers:1.16.2") integrationTestImplementation("org.testcontainers:junit-jupiter:1.16.2") integrationTestImplementation("org.testcontainers:kafka:1.16.2") - integrationTestImplementation("org.hypertrace.core.serviceframework:integrationtest-service-framework:0.1.28") + integrationTestImplementation("org.hypertrace.core.serviceframework:integrationtest-service-framework:0.1.33") integrationTestImplementation("com.github.stefanbirkner:system-lambda:1.2.0") integrationTestImplementation("org.apache.kafka:kafka-clients:5.5.1-ccs") diff --git a/query-service/src/integrationTest/java/org/hypertrace/core/query/service/htqueries/HTPinotQueriesTest.java b/query-service/src/integrationTest/java/org/hypertrace/core/query/service/htqueries/HTPinotQueriesTest.java index 957aa6d8..6c22bee8 100644 --- a/query-service/src/integrationTest/java/org/hypertrace/core/query/service/htqueries/HTPinotQueriesTest.java +++ b/query-service/src/integrationTest/java/org/hypertrace/core/query/service/htqueries/HTPinotQueriesTest.java @@ -96,8 +96,7 @@ public static void setup() throws Exception { new GenericContainer<>(DockerImageName.parse("hypertrace/pinot-servicemanager:main")) .withNetwork(network) .withNetworkAliases("pinot-controller", "pinot-server", "pinot-broker") - .withExposedPorts(8099) - .withExposedPorts(9000) + .withExposedPorts(8099, 9000) .dependsOn(kafkaZk) .withStartupAttempts(CONTAINER_STARTUP_ATTEMPTS) .waitingFor(Wait.forLogMessage(".*Completed schema installation.*", 1))