From 85b18c7d6588f504d811035b5a80f82cebf791e7 Mon Sep 17 00:00:00 2001 From: sarthak Date: Mon, 25 Oct 2021 16:28:28 +0530 Subject: [PATCH] Revert "cleaning up average rate implementation from gateway service (#106)" (#108) This reverts commit a321a68192cebc736f24a1877ae1c6fb5dc33990. --- .../baseline/BaselineServiceQueryParser.java | 9 +- ...tityServiceAndGatewayServiceConverter.java | 13 +- .../QueryAndGatewayDtoConverter.java | 36 +- .../QueryServiceEntityFetcher.java | 17 +- .../common/util/ArithmeticValueUtil.java | 67 ++ .../util/MetricAggregationFunctionUtil.java | 29 + .../validators/function/AvgRateValidator.java | 33 +- .../service/explore/RequestHandler.java | 9 +- .../baseline/BaselineServiceImplTest.java | 95 +-- .../QueryServiceEntityFetcherTests.java | 138 +--- .../common/util/ArithmeticValueUtilTest.java | 161 +++++ .../function/AvgRateValidatorTest.java | 681 ++++++++---------- .../explore/simple-aggregations.json | 2 +- .../explore/simple-aggregations.json | 16 +- 14 files changed, 635 insertions(+), 671 deletions(-) create mode 100644 gateway-service-impl/src/main/java/org/hypertrace/gateway/service/common/util/ArithmeticValueUtil.java create mode 100644 gateway-service-impl/src/test/java/org/hypertrace/gateway/service/common/util/ArithmeticValueUtilTest.java diff --git a/gateway-service-impl/src/main/java/org/hypertrace/gateway/service/baseline/BaselineServiceQueryParser.java b/gateway-service-impl/src/main/java/org/hypertrace/gateway/service/baseline/BaselineServiceQueryParser.java index f1177691..fafcfe0e 100644 --- a/gateway-service-impl/src/main/java/org/hypertrace/gateway/service/baseline/BaselineServiceQueryParser.java +++ b/gateway-service-impl/src/main/java/org/hypertrace/gateway/service/baseline/BaselineServiceQueryParser.java @@ -166,12 +166,13 @@ public BaselineEntitiesResponse parseQueryResponse( } Value convertedValue = - QueryAndGatewayDtoConverter.convertToGatewayValueForMetricValue( - MetricAggregationFunctionUtil.getValueTypeFromFunction( - timeAggregation.getAggregation(), attributeMetadataMap), + MetricAggregationFunctionUtil.getValueFromFunction( + startTime, + endTime, attributeMetadataMap, + row.getColumn(i), metadata, - row.getColumn(i)); + timeAggregation.getAggregation()); BaselineMetricSeries.Builder seriesBuilder = metricSeriesMap.computeIfAbsent( diff --git a/gateway-service-impl/src/main/java/org/hypertrace/gateway/service/common/converters/EntityServiceAndGatewayServiceConverter.java b/gateway-service-impl/src/main/java/org/hypertrace/gateway/service/common/converters/EntityServiceAndGatewayServiceConverter.java index 075e704a..4eac56b6 100644 --- a/gateway-service-impl/src/main/java/org/hypertrace/gateway/service/common/converters/EntityServiceAndGatewayServiceConverter.java +++ b/gateway-service-impl/src/main/java/org/hypertrace/gateway/service/common/converters/EntityServiceAndGatewayServiceConverter.java @@ -19,6 +19,7 @@ import org.hypertrace.gateway.service.common.util.AttributeMetadataUtil; import org.hypertrace.gateway.service.entity.EntitiesRequestContext; import org.hypertrace.gateway.service.entity.config.TimestampConfigs; +import org.hypertrace.gateway.service.v1.common.FunctionType; import org.hypertrace.gateway.service.v1.entity.EntitiesRequest; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -164,10 +165,20 @@ private static Function convertToQueryFunction( .setFunctionName(function.getFunction().name()) .setAlias(function.getAlias()); + // AVGRATE is adding a specific implementation because Pinot does not directly support this + // function switch (function.getFunction()) { case AVGRATE: { - throw new IllegalArgumentException("Doesn't support AVGRATE on entity queries"); + builder.setFunctionName(FunctionType.SUM.name()).setAlias(function.getAlias()); + + // Adds only the argument that is a column identifier for now. + List columns = + function.getArgumentsList().stream() + .filter(org.hypertrace.gateway.service.v1.common.Expression::hasColumnIdentifier) + .collect(Collectors.toList()); + columns.forEach(e -> builder.addArguments(convertToEntityServiceExpression(e))); + break; } case PERCENTILE: { diff --git a/gateway-service-impl/src/main/java/org/hypertrace/gateway/service/common/converters/QueryAndGatewayDtoConverter.java b/gateway-service-impl/src/main/java/org/hypertrace/gateway/service/common/converters/QueryAndGatewayDtoConverter.java index a65f61d6..e43721cd 100644 --- a/gateway-service-impl/src/main/java/org/hypertrace/gateway/service/common/converters/QueryAndGatewayDtoConverter.java +++ b/gateway-service-impl/src/main/java/org/hypertrace/gateway/service/common/converters/QueryAndGatewayDtoConverter.java @@ -1,7 +1,6 @@ package org.hypertrace.gateway.service.common.converters; import com.google.common.base.Strings; -import java.time.Duration; import java.util.List; import java.util.Map; import java.util.stream.Collectors; @@ -19,6 +18,7 @@ import org.hypertrace.core.query.service.api.SortOrder; import org.hypertrace.core.query.service.api.Value; import org.hypertrace.core.query.service.api.ValueType; +import org.hypertrace.gateway.service.v1.common.FunctionType; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -332,26 +332,6 @@ private static LiteralConstant convertToQueryLiteral( .build(); } - private static org.hypertrace.gateway.service.v1.common.Expression - convertLiteralExpressionToIsoDurationString( - org.hypertrace.gateway.service.v1.common.Expression expression) { - // expression can be either long or an iso string - if (expression.getLiteral().getValue().getValueType() - == org.hypertrace.gateway.service.v1.common.ValueType.LONG) { - return org.hypertrace.gateway.service.v1.common.Expression.newBuilder() - .setLiteral( - org.hypertrace.gateway.service.v1.common.LiteralConstant.newBuilder() - .setValue( - org.hypertrace.gateway.service.v1.common.Value.newBuilder() - .setString( - Duration.ofSeconds(expression.getLiteral().getValue().getLong()) - .toString()) - .setValueType(org.hypertrace.gateway.service.v1.common.ValueType.STRING))) - .build(); - } - return expression; - } - private static Function convertToQueryFunction( org.hypertrace.gateway.service.v1.common.FunctionExpression function) { Function.Builder builder = @@ -359,13 +339,19 @@ private static Function convertToQueryFunction( .setFunctionName(function.getFunction().name()) .setAlias(function.getAlias()); + // AVGRATE is adding a specific implementation because Pinot does not directly support this + // function switch (function.getFunction()) { case AVGRATE: { - builder.setFunctionName(function.getFunction().name()).setAlias(function.getAlias()); - function.getArgumentsList().stream() - .map(e -> e.hasLiteral() ? convertLiteralExpressionToIsoDurationString(e) : e) - .forEach(e -> builder.addArguments(convertToQueryExpression(e))); + builder.setFunctionName(FunctionType.SUM.name()).setAlias(function.getAlias()); + + // Adds only the argument that is a column identifier for now. + List columns = + function.getArgumentsList().stream() + .filter(org.hypertrace.gateway.service.v1.common.Expression::hasColumnIdentifier) + .collect(Collectors.toList()); + columns.forEach(e -> builder.addArguments(convertToQueryExpression(e))); break; } case PERCENTILE: diff --git a/gateway-service-impl/src/main/java/org/hypertrace/gateway/service/common/datafetcher/QueryServiceEntityFetcher.java b/gateway-service-impl/src/main/java/org/hypertrace/gateway/service/common/datafetcher/QueryServiceEntityFetcher.java index 3b10ed94..c35dd419 100644 --- a/gateway-service-impl/src/main/java/org/hypertrace/gateway/service/common/datafetcher/QueryServiceEntityFetcher.java +++ b/gateway-service-impl/src/main/java/org/hypertrace/gateway/service/common/datafetcher/QueryServiceEntityFetcher.java @@ -312,11 +312,13 @@ private void addAggregateMetric( Health health = Health.NOT_COMPUTED; Value convertedValue = - QueryAndGatewayDtoConverter.convertToGatewayValueForMetricValue( - MetricAggregationFunctionUtil.getValueTypeFromFunction(function, attributeMetadataMap), + MetricAggregationFunctionUtil.getValueFromFunction( + entitiesRequest.getStartTimeMillis(), + entitiesRequest.getEndTimeMillis(), attributeMetadataMap, + columnValue, metadata, - columnValue); + function); entityBuilder.putMetric( metadata.getColumnName(), @@ -437,12 +439,13 @@ public EntityFetcherResponse getTimeAggregatedMetrics( } Value convertedValue = - QueryAndGatewayDtoConverter.convertToGatewayValueForMetricValue( - MetricAggregationFunctionUtil.getValueTypeFromFunction( - timeAggregation.getAggregation().getFunction(), attributeMetadataMap), + MetricAggregationFunctionUtil.getValueFromFunction( + startTime, + endTime, attributeMetadataMap, + row.getColumn(i), metadata, - row.getColumn(i)); + timeAggregation.getAggregation().getFunction()); List healthExpressions = timeAggregation.getAggregation().getFunction().getArgumentsList().stream() diff --git a/gateway-service-impl/src/main/java/org/hypertrace/gateway/service/common/util/ArithmeticValueUtil.java b/gateway-service-impl/src/main/java/org/hypertrace/gateway/service/common/util/ArithmeticValueUtil.java new file mode 100644 index 00000000..c5979b1a --- /dev/null +++ b/gateway-service-impl/src/main/java/org/hypertrace/gateway/service/common/util/ArithmeticValueUtil.java @@ -0,0 +1,67 @@ +package org.hypertrace.gateway.service.common.util; + +import java.util.concurrent.TimeUnit; +import org.hypertrace.gateway.service.common.converters.QueryAndGatewayDtoConverter; +import org.hypertrace.gateway.service.v1.common.Expression; +import org.hypertrace.gateway.service.v1.common.FunctionExpression; +import org.hypertrace.gateway.service.v1.common.Value; +import org.hypertrace.gateway.service.v1.common.ValueType; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +public class ArithmeticValueUtil { + private static final Logger LOG = LoggerFactory.getLogger(ArithmeticValueUtil.class); + + public static Value divide(Value dividend, Double divisor) { + if (divisor == null || Double.max(divisor, 0) <= 0) { + LOG.error("Attempted division by null or 0"); + throw new IllegalArgumentException("Attempted to divide by 0"); + } + double result = 0.0; + switch (dividend.getValueType()) { + case LONG: + result = ((double) dividend.getLong() / divisor); + break; + case DOUBLE: + result = dividend.getDouble() / divisor; + break; + case STRING: + result = Double.parseDouble(dividend.getString()) / divisor; + break; + case BOOL: + case LONG_ARRAY: + case DOUBLE_ARRAY: + case STRING_ARRAY: + case BOOLEAN_ARRAY: + case TIMESTAMP: + case STRING_MAP: + case UNRECOGNIZED: + LOG.error("Unsupported format for division, requested format: {}", dividend.getValueType()); + throw new IllegalArgumentException( + String.format("Invalid value for divide operation:%s", dividend)); + } + + return Value.newBuilder().setDouble(result).setValueType(ValueType.DOUBLE).build(); + } + + public static Value computeAvgRate( + FunctionExpression originalFunctionExpression, + org.hypertrace.core.query.service.api.Value originalValue, + long startTime, + long endTime) { + Value sumValue = QueryAndGatewayDtoConverter.convertQueryValueToGatewayValue(originalValue); + + // Read the Period from the original expression. If not found throw an exception. This should + // have been validated by the AvgRateValidator. The period should always be set. + Expression periodExpression = + originalFunctionExpression.getArgumentsList().stream() + .filter(org.hypertrace.gateway.service.v1.common.Expression::hasLiteral) + .findFirst() + .orElseThrow(); + + long period = periodExpression.getLiteral().getValue().getLong(); + + Double divisor = ((double) endTime - startTime) / TimeUnit.SECONDS.toMillis(period); + return ArithmeticValueUtil.divide(sumValue, divisor); + } +} diff --git a/gateway-service-impl/src/main/java/org/hypertrace/gateway/service/common/util/MetricAggregationFunctionUtil.java b/gateway-service-impl/src/main/java/org/hypertrace/gateway/service/common/util/MetricAggregationFunctionUtil.java index 16d5a577..18acef17 100644 --- a/gateway-service-impl/src/main/java/org/hypertrace/gateway/service/common/util/MetricAggregationFunctionUtil.java +++ b/gateway-service-impl/src/main/java/org/hypertrace/gateway/service/common/util/MetricAggregationFunctionUtil.java @@ -9,10 +9,13 @@ import org.apache.commons.lang3.tuple.ImmutablePair; import org.hypertrace.core.attribute.service.v1.AttributeKind; import org.hypertrace.core.attribute.service.v1.AttributeMetadata; +import org.hypertrace.core.query.service.api.ColumnMetadata; +import org.hypertrace.gateway.service.common.converters.QueryAndGatewayDtoConverter; import org.hypertrace.gateway.service.v1.common.Expression; import org.hypertrace.gateway.service.v1.common.Expression.ValueCase; import org.hypertrace.gateway.service.v1.common.FunctionExpression; import org.hypertrace.gateway.service.v1.common.FunctionType; +import org.hypertrace.gateway.service.v1.common.Value; /** Class with some utility methods around Aggregated metrics, alias in the entity requests. */ public class MetricAggregationFunctionUtil { @@ -111,4 +114,30 @@ public static AttributeKind getValueTypeFromFunction( return metadata.getValueKind(); } } + + public static Value getValueFromFunction( + long startTime, + long endTime, + Map attributeMetadataMap, + org.hypertrace.core.query.service.api.Value column, + ColumnMetadata metadata, + FunctionExpression functionExpression) { + // AVG_RATE is adding a specific implementation because Pinot does not directly support this + // function, + // so it has to be parsed separately. + Value convertedValue; + if (FunctionType.AVGRATE == functionExpression.getFunction()) { + convertedValue = + ArithmeticValueUtil.computeAvgRate(functionExpression, column, startTime, endTime); + } else { + convertedValue = + QueryAndGatewayDtoConverter.convertToGatewayValueForMetricValue( + MetricAggregationFunctionUtil.getValueTypeFromFunction( + functionExpression, attributeMetadataMap), + attributeMetadataMap, + metadata, + column); + } + return convertedValue; + } } diff --git a/gateway-service-impl/src/main/java/org/hypertrace/gateway/service/common/validators/function/AvgRateValidator.java b/gateway-service-impl/src/main/java/org/hypertrace/gateway/service/common/validators/function/AvgRateValidator.java index f2283d45..69a78374 100644 --- a/gateway-service-impl/src/main/java/org/hypertrace/gateway/service/common/validators/function/AvgRateValidator.java +++ b/gateway-service-impl/src/main/java/org/hypertrace/gateway/service/common/validators/function/AvgRateValidator.java @@ -2,9 +2,6 @@ import static org.hypertrace.gateway.service.v1.common.Expression.ValueCase.HEALTH; -import java.time.Duration; -import java.time.format.DateTimeParseException; -import java.time.temporal.ChronoUnit; import org.hypertrace.gateway.service.v1.common.FunctionExpression; import org.hypertrace.gateway.service.v1.common.FunctionType; import org.hypertrace.gateway.service.v1.common.ValueType; @@ -33,23 +30,12 @@ protected void validateArguments(FunctionExpression functionExpression) { columnIdentifierArgSet = true; break; case LITERAL: - // Need the Period to be set in ISO format - // Added support for backward compatibility so can be long as well (do not use) - + // Need the Period to be set checkArgument( argument.getLiteral().hasValue() - && (argument.getLiteral().getValue().getValueType() == ValueType.STRING - || argument.getLiteral().getValue().getValueType() == ValueType.LONG), - "Period not set as a STRING/LONG Type"); - - long period; - if (argument.getLiteral().getValue().getValueType() == ValueType.STRING) { - String periodInIso = argument.getLiteral().getValue().getString(); - period = isoDurationToSeconds(periodInIso); - } else { - period = argument.getLiteral().getValue().getLong(); - } - + && argument.getLiteral().getValue().getValueType() == ValueType.LONG, + "Period not set as a Long Type"); + Long period = argument.getLiteral().getValue().getLong(); checkArgument(period > 0L, "Period should be > 0"); periodArgSet = true; break; @@ -67,15 +53,4 @@ protected void validateArguments(FunctionExpression functionExpression) { protected Logger getLogger() { return LOG; } - - private static long isoDurationToSeconds(String duration) { - try { - Duration d = java.time.Duration.parse(duration); - return d.get(ChronoUnit.SECONDS); - } catch (DateTimeParseException ex) { - throw new IllegalArgumentException( - String.format( - "Unsupported string format for duration: %s, expects iso string format", duration)); - } - } } diff --git a/gateway-service-impl/src/main/java/org/hypertrace/gateway/service/explore/RequestHandler.java b/gateway-service-impl/src/main/java/org/hypertrace/gateway/service/explore/RequestHandler.java index 63a7e1a0..7cf480b1 100644 --- a/gateway-service-impl/src/main/java/org/hypertrace/gateway/service/explore/RequestHandler.java +++ b/gateway-service-impl/src/main/java/org/hypertrace/gateway/service/explore/RequestHandler.java @@ -281,12 +281,13 @@ void handleQueryServiceResponseSingleColumn( org.hypertrace.gateway.service.v1.common.Value gwValue; if (function != null) { // Function expression value gwValue = - QueryAndGatewayDtoConverter.convertToGatewayValueForMetricValue( - MetricAggregationFunctionUtil.getValueTypeFromFunction( - function, attributeMetadataMap), + MetricAggregationFunctionUtil.getValueFromFunction( + requestContext.getStartTimeMillis(), + requestContext.getEndTimeMillis(), attributeMetadataMap, + queryServiceValue, metadata, - queryServiceValue); + function); } else { // Simple columnId Expression value eg. groupBy columns or column selections gwValue = getValueForColumnIdExpression(queryServiceValue, metadata, attributeMetadataMap); } diff --git a/gateway-service-impl/src/test/java/org/hypertrace/gateway/service/baseline/BaselineServiceImplTest.java b/gateway-service-impl/src/test/java/org/hypertrace/gateway/service/baseline/BaselineServiceImplTest.java index 840eca5a..c8f667b8 100644 --- a/gateway-service-impl/src/test/java/org/hypertrace/gateway/service/baseline/BaselineServiceImplTest.java +++ b/gateway-service-impl/src/test/java/org/hypertrace/gateway/service/baseline/BaselineServiceImplTest.java @@ -10,7 +10,6 @@ import java.util.List; import java.util.Map; import java.util.Optional; -import java.util.stream.Stream; import org.hypertrace.core.attribute.service.v1.AttributeMetadata; import org.hypertrace.core.query.service.api.QueryRequest; import org.hypertrace.core.query.service.api.ResultSetChunk; @@ -31,9 +30,6 @@ import org.hypertrace.gateway.service.v1.common.ValueType; import org.junit.jupiter.api.Assertions; 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.mockito.Mockito; public class BaselineServiceImplTest { @@ -71,7 +67,7 @@ public void testBaselineForEntitiesForAggregates() { Mockito.when( baselineServiceQueryExecutor.executeQuery( Mockito.anyMap(), Mockito.any(QueryRequest.class))) - .thenReturn(getResultSetForAvg("duration_ts").iterator()); + .thenReturn(getResultSet("duration_ts").iterator()); when(entityIdColumnsConfigs.getIdKey("SERVICE")).thenReturn(Optional.of("id")); Map attributeMap = new HashMap<>(); @@ -99,17 +95,17 @@ public void testBaselineForEntitiesForAggregates() { baselineResponse.getBaselineEntityList().get(0).getBaselineAggregateMetricCount() > 0); } - @ParameterizedTest - @MethodSource("getFunctionExpressionForAvgRate") - public void testBaselineEntitiesForAggregatesForAvgRateFunction( - FunctionExpression functionExpression) { + @Test + public void testBaselineEntitiesForAggregatesForAvgRateFunction() { BaselineEntitiesRequest baselineEntitiesRequest = BaselineEntitiesRequest.newBuilder() .setEntityType("SERVICE") .setStartTimeMillis(Instant.parse("2020-11-14T17:40:51.902Z").toEpochMilli()) .setEndTimeMillis(Instant.parse("2020-11-14T18:40:51.902Z").toEpochMilli()) .addEntityIds("entity-1") - .addBaselineAggregateRequest(functionExpression) + .addBaselineAggregateRequest( + getFunctionExpressionForAvgRate( + FunctionType.AVGRATE, "SERVICE.numCalls", "numCalls")) .build(); // Mock section @@ -122,7 +118,7 @@ public void testBaselineEntitiesForAggregatesForAvgRateFunction( Mockito.when( baselineServiceQueryExecutor.executeQuery( Mockito.anyMap(), Mockito.any(QueryRequest.class))) - .thenReturn(getResultSetForAvgRate("numCalls").iterator()); + .thenReturn(getResultSet("numCalls").iterator()); when(entityIdColumnsConfigs.getIdKey("SERVICE")).thenReturn(Optional.of("id")); // Attribute Metadata map contains mapping between Attributes and ID to query data. Map attributeMap = new HashMap<>(); @@ -171,7 +167,7 @@ public void testBaselineEntitiesForMetricSeries() { Mockito.when( baselineServiceQueryExecutor.executeQuery( Mockito.anyMap(), Mockito.any(QueryRequest.class))) - .thenReturn(getResultSetForAvg("duration_ts").iterator()); + .thenReturn(getResultSet("duration_ts").iterator()); Map attributeMap = new HashMap<>(); AttributeMetadata attribute = AttributeMetadata.newBuilder().setFqn("Service.Latency").setId("Service.Id").build(); @@ -220,52 +216,24 @@ private FunctionExpression getFunctionExpressionFor( .build(); } - private static Stream getFunctionExpressionForAvgRate() { - - FunctionType type = FunctionType.AVGRATE; - String columnName = "SERVICE.numCalls"; - String alias = "numCalls"; - - // for iso support - FunctionExpression functionExpression1 = - FunctionExpression.newBuilder() - .setFunction(type) - .setAlias(alias) - .addArguments( - Expression.newBuilder() - .setColumnIdentifier( - ColumnIdentifier.newBuilder().setColumnName(columnName).setAlias(alias))) - .addArguments( - Expression.newBuilder() - .setLiteral( - LiteralConstant.newBuilder() - .setValue( - Value.newBuilder() - .setString("PT1M") - .setValueType(ValueType.STRING)))) - .build(); - - // for long support - FunctionExpression functionExpression2 = - FunctionExpression.newBuilder() - .setFunction(type) - .setAlias(alias) - .addArguments( - Expression.newBuilder() - .setColumnIdentifier( - ColumnIdentifier.newBuilder().setColumnName(columnName).setAlias(alias))) - .addArguments( - Expression.newBuilder() - .setLiteral( - LiteralConstant.newBuilder() - .setValue(Value.newBuilder().setLong(60).setValueType(ValueType.LONG)))) - .build(); - - return Stream.of( - Arguments.arguments(functionExpression1), Arguments.arguments(functionExpression2)); + private FunctionExpression getFunctionExpressionForAvgRate( + FunctionType type, String columnName, String alias) { + return FunctionExpression.newBuilder() + .setFunction(type) + .setAlias(alias) + .addArguments( + Expression.newBuilder() + .setColumnIdentifier( + ColumnIdentifier.newBuilder().setColumnName(columnName).setAlias(alias))) + .addArguments( + Expression.newBuilder() + .setLiteral( + LiteralConstant.newBuilder() + .setValue(Value.newBuilder().setLong(60).setValueType(ValueType.LONG)))) + .build(); } - public List getResultSetForAvg(String alias) { + public List getResultSet(String alias) { long time = Instant.parse("2020-11-14T18:40:51.902Z").toEpochMilli(); return List.of( @@ -280,21 +248,6 @@ public List getResultSetForAvg(String alias) { })); } - public List getResultSetForAvgRate(String alias) { - long time = Instant.parse("2020-11-14T18:40:51.902Z").toEpochMilli(); - - return List.of( - getResultSetChunk( - List.of("SERVICE.id", "dateTimeConvert", alias), - new String[][] { - {"entity-1", String.valueOf(time), "0.3333333333333333"}, - {"entity-1", String.valueOf(time - 60000), "0.6666666666666666"}, - {"entity-1", String.valueOf(time - 120000), "1.0"}, - {"entity-1", String.valueOf(time - 180000), "1.3333333333333333"}, - {"entity-1", String.valueOf(time - 180000), "1.6666666666666667"}, - })); - } - @Test public void testStartTimeCalcGivenTimeRange() { BaselineServiceImpl baselineService = diff --git a/gateway-service-impl/src/test/java/org/hypertrace/gateway/service/common/datafetcher/QueryServiceEntityFetcherTests.java b/gateway-service-impl/src/test/java/org/hypertrace/gateway/service/common/datafetcher/QueryServiceEntityFetcherTests.java index fcdac299..485719ac 100644 --- a/gateway-service-impl/src/test/java/org/hypertrace/gateway/service/common/datafetcher/QueryServiceEntityFetcherTests.java +++ b/gateway-service-impl/src/test/java/org/hypertrace/gateway/service/common/datafetcher/QueryServiceEntityFetcherTests.java @@ -172,8 +172,8 @@ public void testGetTimeAggregatedMetrics() { getResultSetChunk( List.of("API.apiId", "timeColumn", "SUM_API.numCalls", "AVGRATE_API.numCalls"), new String[][] { - {"apiId1", "10000", "34", "68"}, - {"apiId2", "20000", "34", "68"} + {"apiId1", "10000", "34", "34"}, + {"apiId2", "20000", "34", "34"} })); when(queryServiceClient.executeQuery(eq(expectedQueryRequest), eq(requestHeaders), eq(500))) @@ -252,140 +252,6 @@ public void testGetTimeAggregatedMetrics() { new EntityFetcherResponse(expectedEntityKeyBuilderResponseMap), response); } - @Test - public void testGetTimeAggregatedMetricsForAvgRateInIsoFormat() { - - List orderByExpressions = List.of(buildOrderByExpression(API_ID_ATTR)); - long startTime = 1L; - long endTime = 10L; - int limit = 10; - int offset = 5; - String tenantId = "TENANT_ID"; - Map requestHeaders = Map.of("x-tenant-id", tenantId); - AttributeScope entityType = AttributeScope.API; - - TimeAggregation timeAggregation = - buildTimeAggregation( - 30, - API_NUM_CALLS_ATTR, - FunctionType.AVGRATE, - "AVGRATE_API.numCalls", - List.of( - Expression.newBuilder() - .setLiteral( - LiteralConstant.newBuilder() - .setValue( - Value.newBuilder() - .setString("PT1M") - .setValueType(ValueType.STRING))) - .build())); - - EntitiesRequest entitiesRequest = - EntitiesRequest.newBuilder() - .setEntityType(entityType.name()) - .setStartTimeMillis(startTime) - .setEndTimeMillis(endTime) - .addTimeAggregation(timeAggregation) - .setFilter( - Filter.newBuilder() - .setOperator(AND) - .addChildFilter( - EntitiesRequestAndResponseUtils.getTimeRangeFilter( - "API.startTime", startTime, endTime)) - .addChildFilter(generateEQFilter(API_DISCOVERY_STATE_ATTR, "DISCOVERED"))) - .addAllOrderBy(orderByExpressions) - .setLimit(limit) - .setOffset(offset) - .build(); - - EntitiesRequestContext entitiesRequestContext = - new EntitiesRequestContext( - tenantId, startTime, endTime, entityType.name(), "API.startTime", requestHeaders); - - QueryRequest expectedQueryRequest = - QueryRequest.newBuilder() - .addSelection( - QueryAndGatewayDtoConverter.convertToQueryExpression( - timeAggregation.getAggregation())) - .setFilter( - createQsRequestFilter( - API_START_TIME_ATTR, - API_ID_ATTR, - startTime, - endTime, - createStringFilter(API_DISCOVERY_STATE_ATTR, Operator.EQ, "DISCOVERED"))) - .addAllGroupBy( - List.of(API_ID_ATTR).stream() - .map(QueryRequestUtil::createColumnExpression) - .collect(Collectors.toList())) - .addGroupBy(createTimeColumnGroupByExpression(API_START_TIME_ATTR, 30)) - .setOffset(0) - .setLimit(10000) - .build(); - - List resultSetChunks = - List.of( - getResultSetChunk( - List.of("API.apiId", "timeColumn", "AVGRATE_API.numCalls"), - new String[][] { - {"apiId1", "10000", "34"}, - {"apiId2", "20000", "34"} - })); - - when(queryServiceClient.executeQuery(eq(expectedQueryRequest), eq(requestHeaders), eq(500))) - .thenReturn(resultSetChunks.iterator()); - - EntityFetcherResponse response = - queryServiceEntityFetcher.getTimeAggregatedMetrics(entitiesRequestContext, entitiesRequest); - - assertEquals(2, response.size()); - - Map metricSeriesMap1 = new LinkedHashMap<>(); - metricSeriesMap1.put( - "AVGRATE_API.numCalls", - MetricSeries.newBuilder() - .addValue( - Interval.newBuilder() - .setStartTimeMillis(10000) - .setEndTimeMillis(40000) - .setValue(Value.newBuilder().setValueType(ValueType.DOUBLE).setDouble(34.0))) - .setAggregation("AVGRATE") - .setPeriod(Period.newBuilder().setUnit("SECONDS").setValue(30).build()) - .build()); - - Map metricSeriesMap2 = new LinkedHashMap<>(); - metricSeriesMap2.put( - "AVGRATE_API.numCalls", - MetricSeries.newBuilder() - .addValue( - Interval.newBuilder() - .setStartTimeMillis(20000) - .setEndTimeMillis(50000) - .setValue(Value.newBuilder().setValueType(ValueType.DOUBLE).setDouble(34.0))) - .setAggregation("AVGRATE") - .setPeriod(Period.newBuilder().setUnit("SECONDS").setValue(30).build()) - .build()); - - Map expectedEntityKeyBuilderResponseMap = new LinkedHashMap<>(); - expectedEntityKeyBuilderResponseMap.put( - EntityKey.of("apiId1"), - Entity.newBuilder() - .setId("apiId1") - .setEntityType("API") - .putAllMetricSeries(metricSeriesMap1) - .putAttribute("API.id", getStringValue("apiId1"))); - expectedEntityKeyBuilderResponseMap.put( - EntityKey.of("apiId2"), - Entity.newBuilder() - .setId("apiId2") - .setEntityType("API") - .putAllMetricSeries(metricSeriesMap2) - .putAttribute("API.id", getStringValue("apiId2"))); - - compareEntityFetcherResponses( - new EntityFetcherResponse(expectedEntityKeyBuilderResponseMap), response); - } - @Test public void testGetEntities() { List orderByExpressions = List.of(buildOrderByExpression(API_ID_ATTR)); diff --git a/gateway-service-impl/src/test/java/org/hypertrace/gateway/service/common/util/ArithmeticValueUtilTest.java b/gateway-service-impl/src/test/java/org/hypertrace/gateway/service/common/util/ArithmeticValueUtilTest.java new file mode 100644 index 00000000..c0ae3532 --- /dev/null +++ b/gateway-service-impl/src/test/java/org/hypertrace/gateway/service/common/util/ArithmeticValueUtilTest.java @@ -0,0 +1,161 @@ +package org.hypertrace.gateway.service.common.util; + +import java.util.List; +import java.util.NoSuchElementException; +import java.util.concurrent.TimeUnit; +import org.hypertrace.gateway.service.v1.common.ColumnIdentifier; +import org.hypertrace.gateway.service.v1.common.Expression; +import org.hypertrace.gateway.service.v1.common.FunctionExpression; +import org.hypertrace.gateway.service.v1.common.FunctionType; +import org.hypertrace.gateway.service.v1.common.LiteralConstant; +import org.hypertrace.gateway.service.v1.common.Value; +import org.hypertrace.gateway.service.v1.common.ValueType; +import org.junit.jupiter.api.Assertions; +import org.junit.jupiter.api.Test; + +public class ArithmeticValueUtilTest { + + @Test + public void testDivideValid() { + Value expectedResult = + Value.newBuilder().setValueType(ValueType.DOUBLE).setDouble(10.0).build(); + { + Value value = + ArithmeticValueUtil.divide( + Value.newBuilder().setLong(100).setValueType(ValueType.LONG).build(), 10.0); + Assertions.assertEquals(expectedResult, value); + } + + { + Value value = + ArithmeticValueUtil.divide( + Value.newBuilder().setDouble(100.0).setValueType(ValueType.DOUBLE).build(), 10.0); + Assertions.assertEquals(expectedResult, value); + } + + { + Value value = + ArithmeticValueUtil.divide( + Value.newBuilder().setString("100").setValueType(ValueType.STRING).build(), 10.0); + Assertions.assertEquals(expectedResult, value); + } + } + + @Test + public void testDivideInvalid1() { + Assertions.assertThrows( + IllegalArgumentException.class, + () -> { + ArithmeticValueUtil.divide( + Value.newBuilder().setValueType(ValueType.LONG).setLong(100).build(), 0.0); + }); + } + + @Test + public void testDivideInvalid2() { + for (ValueType valueType : + List.of( + ValueType.BOOL, + ValueType.STRING_ARRAY, + ValueType.LONG_ARRAY, + ValueType.BOOLEAN_ARRAY, + ValueType.DOUBLE_ARRAY)) { + try { + ArithmeticValueUtil.divide(Value.newBuilder().setValueType(valueType).build(), 10.0); + } catch (IllegalArgumentException ex) { + // Expected + continue; + } + Assertions.fail("Expecting IllegalArgumentException"); + } + } + + @Test + public void testComputeAvgRateSixtySecondPeriod() { + FunctionExpression functionExpression = + FunctionExpression.newBuilder() + .setFunction(FunctionType.AVGRATE) + .addArguments( + Expression.newBuilder() + .setLiteral( + LiteralConstant.newBuilder() + .setValue(Value.newBuilder().setLong(60).setValueType(ValueType.LONG)))) + .addArguments( + Expression.newBuilder() + .setColumnIdentifier( + ColumnIdentifier.newBuilder().setColumnName("API.bytes_received"))) + .build(); + + org.hypertrace.core.query.service.api.Value originalValue = + org.hypertrace.core.query.service.api.Value.newBuilder() + .setValueType(org.hypertrace.core.query.service.api.ValueType.LONG) + .setLong(120) + .build(); + Value value = + ArithmeticValueUtil.computeAvgRate( + functionExpression, + originalValue, + TimeUnit.SECONDS.toMillis(0), + TimeUnit.SECONDS.toMillis(120)); + Assertions.assertEquals(ValueType.DOUBLE, value.getValueType()); + Assertions.assertEquals(60.0, value.getDouble(), 0.001); + } + + @Test + public void testComputeAvgRateOneSecondPeriod() { + FunctionExpression functionExpression = + FunctionExpression.newBuilder() + .setFunction(FunctionType.AVGRATE) + .addArguments( + Expression.newBuilder() + .setLiteral( + LiteralConstant.newBuilder() + .setValue(Value.newBuilder().setLong(1).setValueType(ValueType.LONG)))) + .addArguments( + Expression.newBuilder() + .setColumnIdentifier( + ColumnIdentifier.newBuilder().setColumnName("API.bytes_received"))) + .build(); + + org.hypertrace.core.query.service.api.Value originalValue = + org.hypertrace.core.query.service.api.Value.newBuilder() + .setValueType(org.hypertrace.core.query.service.api.ValueType.LONG) + .setLong(120) + .build(); + Value value = + ArithmeticValueUtil.computeAvgRate( + functionExpression, + originalValue, + TimeUnit.SECONDS.toMillis(0), + TimeUnit.SECONDS.toMillis(120)); + Assertions.assertEquals(ValueType.DOUBLE, value.getValueType()); + Assertions.assertEquals(1.0, value.getDouble(), 0.001); + } + + @Test + public void testComputeAvgRateNoPeriod() { + FunctionExpression functionExpression = + FunctionExpression.newBuilder() + .setFunction(FunctionType.AVGRATE) + .addArguments( + Expression.newBuilder() + .setColumnIdentifier( + ColumnIdentifier.newBuilder().setColumnName("API.bytes_received"))) + .build(); + + org.hypertrace.core.query.service.api.Value originalValue = + org.hypertrace.core.query.service.api.Value.newBuilder() + .setValueType(org.hypertrace.core.query.service.api.ValueType.LONG) + .setLong(120) + .build(); + Assertions.assertThrows( + NoSuchElementException.class, + () -> { + ArithmeticValueUtil.computeAvgRate( + functionExpression, + originalValue, + TimeUnit.SECONDS.toMillis(0), + TimeUnit.SECONDS.toMillis(120)); + }); + } +} diff --git a/gateway-service-impl/src/test/java/org/hypertrace/gateway/service/common/validators/function/AvgRateValidatorTest.java b/gateway-service-impl/src/test/java/org/hypertrace/gateway/service/common/validators/function/AvgRateValidatorTest.java index 3375f82c..c404e9e7 100644 --- a/gateway-service-impl/src/test/java/org/hypertrace/gateway/service/common/validators/function/AvgRateValidatorTest.java +++ b/gateway-service-impl/src/test/java/org/hypertrace/gateway/service/common/validators/function/AvgRateValidatorTest.java @@ -10,405 +10,320 @@ import org.hypertrace.gateway.service.v1.common.Value; import org.hypertrace.gateway.service.v1.common.ValueType; import org.junit.jupiter.api.Assertions; -import org.junit.jupiter.api.Nested; import org.junit.jupiter.api.Test; public class AvgRateValidatorTest { - @Nested - public class AvgRateValidatorTestForLongFormat { - - @Test - public void validAvgRateFunction_shouldNotThrowIllegalArgException() { - FunctionExpression functionExpression = - FunctionExpression.newBuilder() - .setFunction(FunctionType.AVGRATE) - .setAlias("avg-rate-alias") - .addArguments( - Expression.newBuilder() - .setColumnIdentifier( - ColumnIdentifier.newBuilder().setColumnName("test-column-name"))) - .addArguments( - Expression.newBuilder() - .setLiteral( - LiteralConstant.newBuilder() - .setValue( - Value.newBuilder().setLong(20L).setValueType(ValueType.LONG)))) - .build(); - var avgRateFunctionValidator = new AvgRateValidator(); - avgRateFunctionValidator.validate(functionExpression); - } - - @Test - public void validAvgRateFunctionWithHealth_shouldNotThrowIllegalArgException() { - FunctionExpression functionExpression = - FunctionExpression.newBuilder() - .setFunction(FunctionType.AVGRATE) - .setAlias("avg-rate-alias") - .addArguments( - Expression.newBuilder() - .setColumnIdentifier( - ColumnIdentifier.newBuilder().setColumnName("test-column-name"))) - .addArguments( - Expression.newBuilder() - .setLiteral( - LiteralConstant.newBuilder() - .setValue( - Value.newBuilder().setLong(20L).setValueType(ValueType.LONG)))) - .addArguments(Expression.newBuilder().setHealth(HealthExpression.newBuilder())) - .build(); - var avgRateFunctionValidator = new AvgRateValidator(); - avgRateFunctionValidator.validate(functionExpression); - } - - @Test - public void validAvgRateFunctionWithUnrecognizedArgType_shouldThrowIllegalArgException() { - FunctionExpression functionExpression = - FunctionExpression.newBuilder() - .setFunction(FunctionType.AVGRATE) - .setAlias("avg-rate-alias") - .addArguments( - Expression.newBuilder() - .setColumnIdentifier( - ColumnIdentifier.newBuilder().setColumnName("test-column-name"))) - .addArguments( - Expression.newBuilder() - .setLiteral( - LiteralConstant.newBuilder() - .setValue( - Value.newBuilder().setLong(20L).setValueType(ValueType.LONG)))) - .addArguments(Expression.newBuilder().setOrderBy(OrderByExpression.newBuilder())) - .build(); - var avgRateFunctionValidator = new AvgRateValidator(); - - Assertions.assertThrows( - IllegalArgumentException.class, - () -> { - avgRateFunctionValidator.validate(functionExpression); - }); - } - - @Test - public void nonAvgRateFunction_shouldThrowIllegalArgException() { - FunctionExpression functionExpression = - FunctionExpression.newBuilder() - .setFunction(FunctionType.AVG) - .setAlias("avg-alias") - .addArguments( - Expression.newBuilder() - .setColumnIdentifier( - ColumnIdentifier.newBuilder().setColumnName("test-column-name"))) - .addArguments( - Expression.newBuilder() - .setLiteral( - LiteralConstant.newBuilder() - .setValue( - Value.newBuilder().setLong(20L).setValueType(ValueType.LONG)))) - .build(); - var avgRateFunctionValidator = new AvgRateValidator(); - - Assertions.assertThrows( - IllegalArgumentException.class, - () -> { - avgRateFunctionValidator.validate(functionExpression); - }); - } - - @Test - public void avgRateFunctionWithoutColumnIdentifier_shouldThrowIllegalArgException() { - FunctionExpression functionExpression = - FunctionExpression.newBuilder() - .setFunction(FunctionType.AVGRATE) - .setAlias("avg-rate-alias") - .addArguments( - Expression.newBuilder() - .setLiteral( - LiteralConstant.newBuilder() - .setValue( - Value.newBuilder().setLong(20L).setValueType(ValueType.LONG)))) - .build(); - var avgRateFunctionValidator = new AvgRateValidator(); - - Assertions.assertThrows( - IllegalArgumentException.class, - () -> { - avgRateFunctionValidator.validate(functionExpression); - }); - } - - @Test - public void avgRateFunctionWithoutPeriodAsLiteral_shouldThrowIllegalArgException() { - FunctionExpression functionExpression = - FunctionExpression.newBuilder() - .setFunction(FunctionType.AVGRATE) - .setAlias("avg-rate-alias") - .addArguments( - Expression.newBuilder() - .setColumnIdentifier( - ColumnIdentifier.newBuilder().setColumnName("test-column-name"))) - .build(); - var avgRateFunctionValidator = new AvgRateValidator(); - - Assertions.assertThrows( - IllegalArgumentException.class, - () -> { - avgRateFunctionValidator.validate(functionExpression); - }); - } - - @Test - public void avgRateFunctionWithEmptyColumnName_shouldThrowIllegalArgException() { - FunctionExpression functionExpression = - FunctionExpression.newBuilder() - .setFunction(FunctionType.AVGRATE) - .setAlias("avg-rate-alias") - .addArguments( - Expression.newBuilder() - .setColumnIdentifier(ColumnIdentifier.newBuilder().setColumnName(""))) - .addArguments( - Expression.newBuilder() - .setLiteral( - LiteralConstant.newBuilder() - .setValue( - Value.newBuilder().setLong(20L).setValueType(ValueType.LONG)))) - .build(); - var avgRateFunctionValidator = new AvgRateValidator(); - - Assertions.assertThrows( - IllegalArgumentException.class, - () -> { - avgRateFunctionValidator.validate(functionExpression); - }); - } - - @Test - public void avgRateFunctionWithUnsetColumnName_shouldThrowIllegalArgException() { - FunctionExpression functionExpression = - FunctionExpression.newBuilder() - .setFunction(FunctionType.AVGRATE) - .setAlias("avg-rate-alias") - .addArguments( - Expression.newBuilder().setColumnIdentifier(ColumnIdentifier.newBuilder())) - .addArguments( - Expression.newBuilder() - .setLiteral( - LiteralConstant.newBuilder() - .setValue( - Value.newBuilder().setLong(20L).setValueType(ValueType.LONG)))) - .build(); - var avgRateFunctionValidator = new AvgRateValidator(); - - Assertions.assertThrows( - IllegalArgumentException.class, - () -> { - avgRateFunctionValidator.validate(functionExpression); - }); - } - - @Test - public void avgRateFunctionWithUnsetPeriod_shouldThrowIllegalArgException() { - FunctionExpression functionExpression = - FunctionExpression.newBuilder() - .setFunction(FunctionType.AVGRATE) - .setAlias("avg-rate-alias") - .addArguments( - Expression.newBuilder() - .setColumnIdentifier( - ColumnIdentifier.newBuilder().setColumnName("test-column"))) - .addArguments(Expression.newBuilder().setLiteral(LiteralConstant.newBuilder())) - .build(); - - var avgRateFunctionValidator = new AvgRateValidator(); - - Assertions.assertThrows( - IllegalArgumentException.class, - () -> { - avgRateFunctionValidator.validate(functionExpression); - }); - } - - @Test - public void avgRateFunctionWithNonLongPeriod_shouldThrowIllegalArgException() { - FunctionExpression functionExpression = - FunctionExpression.newBuilder() - .setFunction(FunctionType.AVGRATE) - .setAlias("avg-rate-alias") - .addArguments( - Expression.newBuilder() - .setColumnIdentifier( - ColumnIdentifier.newBuilder().setColumnName("test-column"))) - .addArguments( - Expression.newBuilder() - .setLiteral( - LiteralConstant.newBuilder() - .setValue( - Value.newBuilder() - .setString("not long") - .setValueType(ValueType.STRING)))) - .build(); - var avgRateFunctionValidator = new AvgRateValidator(); + @Test + public void validAvgRateFunction_shouldNotThrowIllegalArgException() { + FunctionExpression functionExpression = + FunctionExpression.newBuilder() + .setFunction(FunctionType.AVGRATE) + .setAlias("avg-rate-alias") + .addArguments( + Expression.newBuilder() + .setColumnIdentifier( + ColumnIdentifier.newBuilder().setColumnName("test-column-name"))) + .addArguments( + Expression.newBuilder() + .setLiteral( + LiteralConstant.newBuilder() + .setValue( + Value.newBuilder().setLong(20L).setValueType(ValueType.LONG)))) + .build(); + var avgRateFunctionValidator = new AvgRateValidator(); + avgRateFunctionValidator.validate(functionExpression); + } - Assertions.assertThrows( - IllegalArgumentException.class, - () -> { - avgRateFunctionValidator.validate(functionExpression); - }); - } + @Test + public void validAvgRateFunctionWithHealth_shouldNotThrowIllegalArgException() { + FunctionExpression functionExpression = + FunctionExpression.newBuilder() + .setFunction(FunctionType.AVGRATE) + .setAlias("avg-rate-alias") + .addArguments( + Expression.newBuilder() + .setColumnIdentifier( + ColumnIdentifier.newBuilder().setColumnName("test-column-name"))) + .addArguments( + Expression.newBuilder() + .setLiteral( + LiteralConstant.newBuilder() + .setValue( + Value.newBuilder().setLong(20L).setValueType(ValueType.LONG)))) + .addArguments(Expression.newBuilder().setHealth(HealthExpression.newBuilder())) + .build(); + var avgRateFunctionValidator = new AvgRateValidator(); + avgRateFunctionValidator.validate(functionExpression); + } - @Test - public void avgRateFunctionWithZeroPeriod_shouldThrowIllegalArgException() { - FunctionExpression functionExpression = - FunctionExpression.newBuilder() - .setFunction(FunctionType.AVGRATE) - .setAlias("avg-rate-alias") - .addArguments( - Expression.newBuilder() - .setColumnIdentifier( - ColumnIdentifier.newBuilder().setColumnName("test-column"))) - .addArguments( - Expression.newBuilder() - .setLiteral( - LiteralConstant.newBuilder() - .setValue( - Value.newBuilder().setLong(0L).setValueType(ValueType.LONG)))) - .build(); - var avgRateFunctionValidator = new AvgRateValidator(); + @Test + public void validAvgRateFunctionWithUnrecognizedArgType_shouldThrowIllegalArgException() { + FunctionExpression functionExpression = + FunctionExpression.newBuilder() + .setFunction(FunctionType.AVGRATE) + .setAlias("avg-rate-alias") + .addArguments( + Expression.newBuilder() + .setColumnIdentifier( + ColumnIdentifier.newBuilder().setColumnName("test-column-name"))) + .addArguments( + Expression.newBuilder() + .setLiteral( + LiteralConstant.newBuilder() + .setValue( + Value.newBuilder().setLong(20L).setValueType(ValueType.LONG)))) + .addArguments(Expression.newBuilder().setOrderBy(OrderByExpression.newBuilder())) + .build(); + var avgRateFunctionValidator = new AvgRateValidator(); + + Assertions.assertThrows( + IllegalArgumentException.class, + () -> { + avgRateFunctionValidator.validate(functionExpression); + }); + } - Assertions.assertThrows( - IllegalArgumentException.class, - () -> { - avgRateFunctionValidator.validate(functionExpression); - }); - } + @Test + public void nonAvgRateFunction_shouldThrowIllegalArgException() { + FunctionExpression functionExpression = + FunctionExpression.newBuilder() + .setFunction(FunctionType.AVG) + .setAlias("avg-alias") + .addArguments( + Expression.newBuilder() + .setColumnIdentifier( + ColumnIdentifier.newBuilder().setColumnName("test-column-name"))) + .addArguments( + Expression.newBuilder() + .setLiteral( + LiteralConstant.newBuilder() + .setValue( + Value.newBuilder().setLong(20L).setValueType(ValueType.LONG)))) + .build(); + var avgRateFunctionValidator = new AvgRateValidator(); + + Assertions.assertThrows( + IllegalArgumentException.class, + () -> { + avgRateFunctionValidator.validate(functionExpression); + }); + } - @Test - public void avgRateFunctionWithNoAlias_shouldThrowIllegalArgException() { - FunctionExpression functionExpression = - FunctionExpression.newBuilder() - .setFunction(FunctionType.AVGRATE) - .addArguments( - Expression.newBuilder() - .setColumnIdentifier( - ColumnIdentifier.newBuilder().setColumnName("test-column"))) - .addArguments( - Expression.newBuilder() - .setLiteral( - LiteralConstant.newBuilder() - .setValue( - Value.newBuilder().setLong(20L).setValueType(ValueType.LONG)))) - .build(); - var avgRateFunctionValidator = new AvgRateValidator(); + @Test + public void avgRateFunctionWithoutColumnIdentifier_shouldThrowIllegalArgException() { + FunctionExpression functionExpression = + FunctionExpression.newBuilder() + .setFunction(FunctionType.AVGRATE) + .setAlias("avg-rate-alias") + .addArguments( + Expression.newBuilder() + .setLiteral( + LiteralConstant.newBuilder() + .setValue( + Value.newBuilder().setLong(20L).setValueType(ValueType.LONG)))) + .build(); + var avgRateFunctionValidator = new AvgRateValidator(); + + Assertions.assertThrows( + IllegalArgumentException.class, + () -> { + avgRateFunctionValidator.validate(functionExpression); + }); + } - Assertions.assertThrows( - IllegalArgumentException.class, - () -> { - avgRateFunctionValidator.validate(functionExpression); - }); - } + @Test + public void avgRateFunctionWithoutPeriodAsLiteral_shouldThrowIllegalArgException() { + FunctionExpression functionExpression = + FunctionExpression.newBuilder() + .setFunction(FunctionType.AVGRATE) + .setAlias("avg-rate-alias") + .addArguments( + Expression.newBuilder() + .setColumnIdentifier( + ColumnIdentifier.newBuilder().setColumnName("test-column-name"))) + .build(); + var avgRateFunctionValidator = new AvgRateValidator(); + + Assertions.assertThrows( + IllegalArgumentException.class, + () -> { + avgRateFunctionValidator.validate(functionExpression); + }); + } - @Test - public void avgRateFunctionWithEmptyAlias_shouldThrowIllegalArgException() { - FunctionExpression functionExpression = - FunctionExpression.newBuilder() - .setFunction(FunctionType.AVGRATE) - .setAlias("") - .addArguments( - Expression.newBuilder() - .setColumnIdentifier( - ColumnIdentifier.newBuilder().setColumnName("test-column"))) - .addArguments( - Expression.newBuilder() - .setLiteral( - LiteralConstant.newBuilder() - .setValue( - Value.newBuilder().setLong(20L).setValueType(ValueType.LONG)))) - .build(); - var avgRateFunctionValidator = new AvgRateValidator(); + @Test + public void avgRateFunctionWithEmptyColumnName_shouldThrowIllegalArgException() { + FunctionExpression functionExpression = + FunctionExpression.newBuilder() + .setFunction(FunctionType.AVGRATE) + .setAlias("avg-rate-alias") + .addArguments( + Expression.newBuilder() + .setColumnIdentifier(ColumnIdentifier.newBuilder().setColumnName(""))) + .addArguments( + Expression.newBuilder() + .setLiteral( + LiteralConstant.newBuilder() + .setValue( + Value.newBuilder().setLong(20L).setValueType(ValueType.LONG)))) + .build(); + var avgRateFunctionValidator = new AvgRateValidator(); + + Assertions.assertThrows( + IllegalArgumentException.class, + () -> { + avgRateFunctionValidator.validate(functionExpression); + }); + } - Assertions.assertThrows( - IllegalArgumentException.class, - () -> { - avgRateFunctionValidator.validate(functionExpression); - }); - } + @Test + public void avgRateFunctionWithUnsetColumnName_shouldThrowIllegalArgException() { + FunctionExpression functionExpression = + FunctionExpression.newBuilder() + .setFunction(FunctionType.AVGRATE) + .setAlias("avg-rate-alias") + .addArguments( + Expression.newBuilder().setColumnIdentifier(ColumnIdentifier.newBuilder())) + .addArguments( + Expression.newBuilder() + .setLiteral( + LiteralConstant.newBuilder() + .setValue( + Value.newBuilder().setLong(20L).setValueType(ValueType.LONG)))) + .build(); + var avgRateFunctionValidator = new AvgRateValidator(); + + Assertions.assertThrows( + IllegalArgumentException.class, + () -> { + avgRateFunctionValidator.validate(functionExpression); + }); } - @Nested - public class AvgRateValidatorTestForIsoFormat { + @Test + public void avgRateFunctionWithUnsetPeriod_shouldThrowIllegalArgException() { + FunctionExpression functionExpression = + FunctionExpression.newBuilder() + .setFunction(FunctionType.AVGRATE) + .setAlias("avg-rate-alias") + .addArguments( + Expression.newBuilder() + .setColumnIdentifier( + ColumnIdentifier.newBuilder().setColumnName("test-column"))) + .addArguments(Expression.newBuilder().setLiteral(LiteralConstant.newBuilder())) + .build(); + + var avgRateFunctionValidator = new AvgRateValidator(); + + Assertions.assertThrows( + IllegalArgumentException.class, + () -> { + avgRateFunctionValidator.validate(functionExpression); + }); + } - @Test - public void validAvgRateFunction_shouldNotThrowIllegalArgException() { - FunctionExpression functionExpression = - FunctionExpression.newBuilder() - .setFunction(FunctionType.AVGRATE) - .setAlias("avg-rate-alias") - .addArguments( - Expression.newBuilder() - .setColumnIdentifier( - ColumnIdentifier.newBuilder().setColumnName("test-column-name"))) - .addArguments( - Expression.newBuilder() - .setLiteral( - LiteralConstant.newBuilder() - .setValue( - Value.newBuilder() - .setString("PT1S") - .setValueType(ValueType.STRING)))) - .build(); - var avgRateFunctionValidator = new AvgRateValidator(); - avgRateFunctionValidator.validate(functionExpression); - } + @Test + public void avgRateFunctionWithNonLongPeriod_shouldThrowIllegalArgException() { + FunctionExpression functionExpression = + FunctionExpression.newBuilder() + .setFunction(FunctionType.AVGRATE) + .setAlias("avg-rate-alias") + .addArguments( + Expression.newBuilder() + .setColumnIdentifier( + ColumnIdentifier.newBuilder().setColumnName("test-column"))) + .addArguments( + Expression.newBuilder() + .setLiteral( + LiteralConstant.newBuilder() + .setValue( + Value.newBuilder() + .setString("not long") + .setValueType(ValueType.STRING)))) + .build(); + var avgRateFunctionValidator = new AvgRateValidator(); + + Assertions.assertThrows( + IllegalArgumentException.class, + () -> { + avgRateFunctionValidator.validate(functionExpression); + }); + } - @Test - public void validAvgRateFunctionWithHealth_shouldNotThrowIllegalArgException() { - FunctionExpression functionExpression = - FunctionExpression.newBuilder() - .setFunction(FunctionType.AVGRATE) - .setAlias("avg-rate-alias") - .addArguments( - Expression.newBuilder() - .setColumnIdentifier( - ColumnIdentifier.newBuilder().setColumnName("test-column-name"))) - .addArguments( - Expression.newBuilder() - .setLiteral( - LiteralConstant.newBuilder() - .setValue( - Value.newBuilder() - .setString("PT1S") - .setValueType(ValueType.STRING)))) - .addArguments(Expression.newBuilder().setHealth(HealthExpression.newBuilder())) - .build(); - var avgRateFunctionValidator = new AvgRateValidator(); - avgRateFunctionValidator.validate(functionExpression); - } + @Test + public void avgRateFunctionWithZeroPeriod_shouldThrowIllegalArgException() { + FunctionExpression functionExpression = + FunctionExpression.newBuilder() + .setFunction(FunctionType.AVGRATE) + .setAlias("avg-rate-alias") + .addArguments( + Expression.newBuilder() + .setColumnIdentifier( + ColumnIdentifier.newBuilder().setColumnName("test-column"))) + .addArguments( + Expression.newBuilder() + .setLiteral( + LiteralConstant.newBuilder() + .setValue(Value.newBuilder().setLong(0L).setValueType(ValueType.LONG)))) + .build(); + var avgRateFunctionValidator = new AvgRateValidator(); + + Assertions.assertThrows( + IllegalArgumentException.class, + () -> { + avgRateFunctionValidator.validate(functionExpression); + }); + } - @Test - public void avgRateFunctionWithZeroPeriod_shouldThrowIllegalArgException() { - FunctionExpression functionExpression = - FunctionExpression.newBuilder() - .setFunction(FunctionType.AVGRATE) - .setAlias("avg-rate-alias") - .addArguments( - Expression.newBuilder() - .setColumnIdentifier( - ColumnIdentifier.newBuilder().setColumnName("test-column"))) - .addArguments( - Expression.newBuilder() - .setLiteral( - LiteralConstant.newBuilder() - .setValue( - Value.newBuilder() - .setString("PT0S") - .setValueType(ValueType.STRING)))) - .build(); - var avgRateFunctionValidator = new AvgRateValidator(); + @Test + public void avgRateFunctionWithNoAlias_shouldThrowIllegalArgException() { + FunctionExpression functionExpression = + FunctionExpression.newBuilder() + .setFunction(FunctionType.AVGRATE) + .addArguments( + Expression.newBuilder() + .setColumnIdentifier( + ColumnIdentifier.newBuilder().setColumnName("test-column"))) + .addArguments( + Expression.newBuilder() + .setLiteral( + LiteralConstant.newBuilder() + .setValue( + Value.newBuilder().setLong(20L).setValueType(ValueType.LONG)))) + .build(); + var avgRateFunctionValidator = new AvgRateValidator(); + + Assertions.assertThrows( + IllegalArgumentException.class, + () -> { + avgRateFunctionValidator.validate(functionExpression); + }); + } - Assertions.assertThrows( - IllegalArgumentException.class, - () -> { - avgRateFunctionValidator.validate(functionExpression); - }); - } + @Test + public void avgRateFunctionWithEmptyAlias_shouldThrowIllegalArgException() { + FunctionExpression functionExpression = + FunctionExpression.newBuilder() + .setFunction(FunctionType.AVGRATE) + .setAlias("") + .addArguments( + Expression.newBuilder() + .setColumnIdentifier( + ColumnIdentifier.newBuilder().setColumnName("test-column"))) + .addArguments( + Expression.newBuilder() + .setLiteral( + LiteralConstant.newBuilder() + .setValue( + Value.newBuilder().setLong(20L).setValueType(ValueType.LONG)))) + .build(); + var avgRateFunctionValidator = new AvgRateValidator(); + + Assertions.assertThrows( + IllegalArgumentException.class, + () -> { + avgRateFunctionValidator.validate(functionExpression); + }); } } diff --git a/gateway-service-impl/src/test/resources/expected-test-responses/explore/simple-aggregations.json b/gateway-service-impl/src/test/resources/expected-test-responses/explore/simple-aggregations.json index 9835e0e5..7d17da51 100644 --- a/gateway-service-impl/src/test/resources/expected-test-responses/explore/simple-aggregations.json +++ b/gateway-service-impl/src/test/resources/expected-test-responses/explore/simple-aggregations.json @@ -16,7 +16,7 @@ }, "RATE#results/rateLatency:Api.Trace|duration": { "valueType": "DOUBLE", - "double": 8594971.0 + "double": 4774.983888888889 } } } diff --git a/gateway-service-impl/src/test/resources/query-service-requests-and-responses/explore/simple-aggregations.json b/gateway-service-impl/src/test/resources/query-service-requests-and-responses/explore/simple-aggregations.json index 0937c402..a5ee3dcf 100644 --- a/gateway-service-impl/src/test/resources/query-service-requests-and-responses/explore/simple-aggregations.json +++ b/gateway-service-impl/src/test/resources/query-service-requests-and-responses/explore/simple-aggregations.json @@ -102,18 +102,14 @@ }, { "function": { - "functionName": "AVGRATE", - "arguments": [{ - "columnIdentifier": { - "columnName": "API_TRACE.duration" - } - }, { - "literal": { - "value": { - "string": "PT2S" + "functionName": "SUM", + "arguments": [ + { + "columnIdentifier": { + "columnName": "API_TRACE.duration" } } - }], + ], "alias": "RATE#results/rateLatency:Api.Trace|duration" } }