Skip to content
This repository has been archived by the owner on Jun 26, 2024. It is now read-only.

Commit

Permalink
Revert "cleaning up average rate implementation from gateway service (#…
Browse files Browse the repository at this point in the history
…106)" (#108)

This reverts commit a321a68.
  • Loading branch information
sarthak77 authored Oct 25, 2021
1 parent a321a68 commit 85b18c7
Show file tree
Hide file tree
Showing 14 changed files with 635 additions and 671 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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<org.hypertrace.gateway.service.v1.common.Expression> 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:
{
Expand Down
Original file line number Diff line number Diff line change
@@ -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;
Expand All @@ -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;

Expand Down Expand Up @@ -332,40 +332,26 @@ 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 =
Function.newBuilder()
.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<org.hypertrace.gateway.service.v1.common.Expression> 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:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
Expand Down Expand Up @@ -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<org.hypertrace.gateway.service.v1.common.Expression> healthExpressions =
timeAggregation.getAggregation().getFunction().getArgumentsList().stream()
Expand Down
Original file line number Diff line number Diff line change
@@ -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);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -111,4 +114,30 @@ public static AttributeKind getValueTypeFromFunction(
return metadata.getValueKind();
}
}

public static Value getValueFromFunction(
long startTime,
long endTime,
Map<String, AttributeMetadata> 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;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand All @@ -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));
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand Down
Loading

0 comments on commit 85b18c7

Please sign in to comment.