feat: Add promql query converter and request handler #111
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,14 @@ | ||
| package org.hypertrace.core.query.service; | ||
|
|
||
| import java.time.Duration; | ||
| import java.time.Instant; | ||
| import lombok.AllArgsConstructor; | ||
| import lombok.Value; | ||
|
|
||
| @Value | ||
| @AllArgsConstructor | ||
| public class QueryTimeRange { | ||
|
findingrish marked this conversation as resolved.
|
||
| Instant startTime; | ||
| Instant endTime; | ||
| Duration duration; | ||
| } | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,112 @@ | ||
| package org.hypertrace.core.query.service.prometheus; | ||
|
|
||
| import static org.hypertrace.core.query.service.QueryRequestUtil.getLogicalColumnNameForSimpleColumnExpression; | ||
|
|
||
| import com.google.protobuf.ByteString; | ||
| import java.util.List; | ||
| import java.util.function.Function; | ||
| import org.apache.commons.codec.binary.Hex; | ||
| import org.hypertrace.core.query.service.QueryRequestUtil; | ||
| import org.hypertrace.core.query.service.api.Expression; | ||
| import org.hypertrace.core.query.service.api.Filter; | ||
| import org.hypertrace.core.query.service.api.LiteralConstant; | ||
| import org.hypertrace.core.query.service.api.Operator; | ||
| import org.hypertrace.core.query.service.api.Value; | ||
|
|
||
| class FilterToPromqlConverter { | ||
|
|
||
| /** only `AND` operator in filter is allowed rhs of leaf filter should be literal */ | ||
| void convertFilterToString( | ||
| Filter filter, | ||
| String timeFilterColumn, | ||
| Function<Expression, String> expressionToColumnConverter, | ||
| List<String> filterList) { | ||
| if (filter.getChildFilterCount() > 0) { | ||
| for (Filter childFilter : filter.getChildFilterList()) { | ||
| convertFilterToString( | ||
| childFilter, timeFilterColumn, expressionToColumnConverter, filterList); | ||
| } | ||
| } else { | ||
| if (QueryRequestUtil.isSimpleColumnExpression(filter.getLhs()) | ||
|
Contributor
There was a problem hiding this comment.
|
||
| && timeFilterColumn.equals( | ||
| getLogicalColumnNameForSimpleColumnExpression(filter.getLhs()))) { | ||
| return; | ||
| } | ||
| StringBuilder builder = new StringBuilder(); | ||
| builder.append(expressionToColumnConverter.apply(filter.getLhs())); | ||
| builder.append(convertOperatorToString(filter.getOperator())); | ||
| builder.append(convertLiteralToString(filter.getRhs().getLiteral())); | ||
|
Contributor
There was a problem hiding this comment. What will the null value
Author
There was a problem hiding this comment. Throwing exception from convertLiteralToString |
||
| filterList.add(builder.toString()); | ||
| } | ||
| } | ||
|
|
||
| private String convertOperatorToString(Operator operator) { | ||
| switch (operator) { | ||
| case IN: | ||
| case EQ: | ||
| return "="; | ||
| case NEQ: | ||
| return "!="; | ||
| case LIKE: | ||
| return "=~"; | ||
| default: | ||
| throw new RuntimeException( | ||
| String.format("Equivalent %s operator not supported in promql", operator)); | ||
| } | ||
| } | ||
|
|
||
| private String convertLiteralToString(LiteralConstant literal) { | ||
| Value value = literal.getValue(); | ||
| switch (value.getValueType()) { | ||
| case STRING_ARRAY: | ||
| StringBuilder builder = new StringBuilder("\""); | ||
| for (String item : value.getStringArrayList()) { | ||
| if (builder.length() > 1) { | ||
| builder.append("|"); | ||
| } | ||
| builder.append(item); | ||
| } | ||
| builder.append("\""); | ||
| return builder.toString(); | ||
| case BYTES_ARRAY: | ||
| builder = new StringBuilder("\""); | ||
| for (ByteString item : value.getBytesArrayList()) { | ||
| if (builder.length() > 1) { | ||
| builder.append("|"); | ||
| } | ||
| builder.append(Hex.encodeHexString(item.toByteArray())); | ||
| } | ||
| builder.append("\""); | ||
| return builder.toString(); | ||
| case STRING: | ||
| return "\"" + value.getString() + "\""; | ||
| case LONG: | ||
| return "\"" + value.getLong() + "\""; | ||
| case INT: | ||
| return "\"" + value.getInt() + "\""; | ||
| case FLOAT: | ||
| return "\"" + value.getFloat() + "\""; | ||
| case DOUBLE: | ||
| return "\"" + value.getDouble() + "\""; | ||
| case BYTES: | ||
| return "\"" + Hex.encodeHexString(value.getBytes().toByteArray()) + "\""; | ||
| case BOOL: | ||
| return "\"" + value.getBoolean() + "\""; | ||
| case TIMESTAMP: | ||
| return "\"" + value.getTimestamp() + "\""; | ||
| case NULL_NUMBER: | ||
| return "0"; | ||
| case NULL_STRING: | ||
| return "null"; | ||
| case LONG_ARRAY: | ||
| case INT_ARRAY: | ||
| case FLOAT_ARRAY: | ||
| case DOUBLE_ARRAY: | ||
| case BOOLEAN_ARRAY: | ||
| case UNRECOGNIZED: | ||
| default: | ||
| throw new RuntimeException( | ||
| String.format("Literal type %s not supported", value.getValueType())); | ||
| } | ||
| } | ||
| } | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,86 @@ | ||
| package org.hypertrace.core.query.service.prometheus; | ||
|
|
||
| import com.google.common.base.Preconditions; | ||
| import com.typesafe.config.Config; | ||
| import io.reactivex.rxjava3.core.Observable; | ||
| import java.util.Optional; | ||
| import org.hypertrace.core.query.service.ExecutionContext; | ||
| import org.hypertrace.core.query.service.QueryCost; | ||
| import org.hypertrace.core.query.service.RequestHandler; | ||
| import org.hypertrace.core.query.service.api.QueryRequest; | ||
| import org.hypertrace.core.query.service.api.Row; | ||
|
|
||
| public class PrometheusBasedRequestHandler implements RequestHandler { | ||
|
|
||
| private static final String VIEW_DEFINITION_CONFIG_KEY = "prometheusViewDefinition"; | ||
| private static final String TENANT_ATTRIBUTE_NAME_CONFIG_KEY = "tenantAttributeName"; | ||
| private static final String START_TIME_ATTRIBUTE_NAME_CONFIG_KEY = "startTimeAttributeName"; | ||
|
|
||
| private final QueryRequestEligibilityValidator queryRequestEligibilityValidator; | ||
| private final String name; | ||
| private PrometheusViewDefinition prometheusViewDefinition; | ||
| private Optional<String> startTimeAttributeName; | ||
| private QueryRequestToPromqlConverter requestToPromqlConverter; | ||
|
|
||
| PrometheusBasedRequestHandler(String name, Config config) { | ||
| this.name = name; | ||
| this.processConfig(config); | ||
| this.queryRequestEligibilityValidator = | ||
| new QueryRequestEligibilityValidator(prometheusViewDefinition); | ||
| } | ||
|
|
||
| @Override | ||
| public String getName() { | ||
| return name; | ||
| } | ||
|
|
||
| @Override | ||
| public Optional<String> getTimeFilterColumn() { | ||
| return this.startTimeAttributeName; | ||
| } | ||
|
|
||
| private void processConfig(Config config) { | ||
|
|
||
| if (!config.hasPath(TENANT_ATTRIBUTE_NAME_CONFIG_KEY)) { | ||
| throw new RuntimeException( | ||
| TENANT_ATTRIBUTE_NAME_CONFIG_KEY | ||
| + " is not defined in the " | ||
| + name | ||
| + " request handler."); | ||
| } | ||
|
|
||
| String tenantAttributeName = config.getString(TENANT_ATTRIBUTE_NAME_CONFIG_KEY); | ||
| this.prometheusViewDefinition = | ||
| PrometheusViewDefinition.parse( | ||
| config.getConfig(VIEW_DEFINITION_CONFIG_KEY), tenantAttributeName); | ||
|
|
||
| this.startTimeAttributeName = | ||
| config.hasPath(START_TIME_ATTRIBUTE_NAME_CONFIG_KEY) | ||
| ? Optional.of(config.getString(START_TIME_ATTRIBUTE_NAME_CONFIG_KEY)) | ||
| : Optional.empty(); | ||
|
|
||
| this.requestToPromqlConverter = new QueryRequestToPromqlConverter(prometheusViewDefinition); | ||
| } | ||
|
|
||
| /** | ||
| * Returns a QueryCost that is an indication of whether the given query can be handled by this | ||
| * handler and if so, how costly is it to handle that query. | ||
| */ | ||
| @Override | ||
| public QueryCost canHandle(QueryRequest request, ExecutionContext executionContext) { | ||
| return queryRequestEligibilityValidator.calculateCost(request, executionContext); | ||
| } | ||
|
|
||
| @Override | ||
| public Observable<Row> handleRequest( | ||
| QueryRequest originalRequest, ExecutionContext executionContext) { | ||
|
|
||
| // Validate QueryContext and tenant id presence | ||
| Preconditions.checkNotNull(executionContext); | ||
| Preconditions.checkNotNull(executionContext.getTenantId()); | ||
|
|
||
| // todo call convert and execute request using client here | ||
|
|
||
| return null; | ||
| } | ||
| } |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would have been niced to abstract this pinot detail (like we did with AVGRATE) before adding promql support
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you please elaborate more (for AVGRATE, wasn't the work around adding support in qs)?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So the concept of AVGRATE, similar to grouping by time is an abstract one that we have exposed in our own query language. In the original implementation (for both), the translation from that abstract concept to a pinot specific implementation happened in gateway which was a leak of implementation details that should be part of query service's abstraction. AVGRATE since was pushed down, so now gateway just asks for avgrate and we translate that to it's pinot equivalent (and eventually, I assume, it's promQL equivalent). Similarly, dateTimeConvert should do the same.
This may be a fair bit of work, so it's up to you guys on the approach, but what we're forcing ourselves to do right now is translating it first from our own language to a pinot expression, then from that pinot expression back to an abstraction.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Filed #112