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

Commit

Permalink
fix: add support for reversing explore time series request (#82)
Browse files Browse the repository at this point in the history 
* fix: add support for reverting explore time series request

* chore: update vulns
  • Loading branch information
@aaron-steinfeld
aaron-steinfeld authored Mar 13, 2021
1 parent 9ed0f73 commit 3ff26d4
Show file tree
Hide file tree
Showing 6 changed files with 99 additions and 48 deletions.
2 changes: 1 addition & 1 deletion .snyk
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,6 @@ ignore:
SNYK-JAVA-IONETTY-1042268:
- '*':
reason: No replacement available
expires: 2021-02-28T00:00:00.000Z
expires: 2021-04-30T00:00:00.000Z

patch: {}
6 changes: 3 additions & 3 deletions gateway-service-api/build.gradle.kts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,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.32.1"
artifact = "io.grpc:protoc-gen-grpc-java:1.36.0"
}
}
generateProtoTasks {
Expand All @@ -42,9 +42,9 @@ sourceSets {
}

dependencies {
api("io.grpc:grpc-protobuf:1.33.0")
api("io.grpc:grpc-protobuf:1.36.0")
api("com.google.api.grpc:proto-google-common-protos:1.18.1")
api("io.grpc:grpc-stub:1.33.0")
api("io.grpc:grpc-stub:1.36.0")
api("javax.annotation:javax.annotation-api:1.3.2")

constraints {
Expand Down
18 changes: 3 additions & 15 deletions gateway-service-impl/build.gradle.kts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,24 +11,12 @@ tasks.test {
dependencies {
api(project(":gateway-service-api"))

constraints {
implementation("com.google.guava:guava:30.0-jre") {
because("Information Disclosure [Medium Severity][https://snyk.io/vuln/SNYK-JAVA-COMGOOGLEGUAVA-1015415] in com.google.guava:[email protected]")
}
testRuntimeOnly("io.netty:netty-codec-http2:4.1.53.Final") {
because("https://snyk.io/vuln/SNYK-JAVA-IONETTY-1020439")
}
testRuntimeOnly("io.netty:netty-handler-proxy:4.1.53.Final") {
because("https://snyk.io/vuln/SNYK-JAVA-IONETTY-1020439s")
}
}

implementation("org.hypertrace.core.query.service:query-service-client:0.5.2")
implementation("org.hypertrace.core.attribute.service:attribute-service-client:0.9.3")
implementation("org.hypertrace.entity.service:entity-service-client:0.5.6")
implementation("org.hypertrace.entity.service:entity-service-api:0.5.6")
implementation("org.hypertrace.core.grpcutils:grpc-context-utils:0.3.2")
implementation("org.hypertrace.core.serviceframework:platform-metrics:0.1.19")
implementation("org.hypertrace.core.grpcutils:grpc-context-utils:0.3.4")
implementation("org.hypertrace.core.serviceframework:platform-metrics:0.1.21")

// Config
implementation("com.typesafe:config:1.4.1")
Expand All @@ -45,5 +33,5 @@ dependencies {
testImplementation("org.junit.jupiter:junit-jupiter:5.7.0")
testImplementation("org.mockito:mockito-core:3.6.28")
testImplementation("org.apache.logging.log4j:log4j-slf4j-impl:2.14.0")
testImplementation("io.grpc:grpc-netty:1.33.1")
testImplementation("io.grpc:grpc-netty:1.36.0")
}
59 changes: 37 additions & 22 deletions .../src/main/java/org/hypertrace/gateway/service/explore/TimeAggregationsRequestHandler.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,13 @@
import org.hypertrace.gateway.service.common.converters.QueryAndGatewayDtoConverter;
import org.hypertrace.gateway.service.common.util.AttributeMetadataUtil;
import org.hypertrace.gateway.service.common.util.QueryExpressionUtil;
import org.hypertrace.gateway.service.v1.common.ColumnIdentifier;
import org.hypertrace.gateway.service.v1.common.Expression;
import org.hypertrace.gateway.service.v1.common.OrderByExpression;
import org.hypertrace.gateway.service.v1.common.Period;
import org.hypertrace.gateway.service.v1.common.SortOrder;
import org.hypertrace.gateway.service.v1.common.TimeAggregation;
import org.hypertrace.gateway.service.v1.common.ValueType;
import org.hypertrace.gateway.service.v1.explore.ColumnName;
import org.hypertrace.gateway.service.v1.explore.ExploreRequest;
import org.hypertrace.gateway.service.v1.explore.ExploreResponse;
Expand Down Expand Up @@ -80,30 +85,40 @@ QueryRequest buildQueryRequest(
* @return
*/
@Override
public List<org.hypertrace.gateway.service.v1.common.OrderByExpression>
public List<OrderByExpression>
getRequestOrderByExpressions(ExploreRequest request) {
List<org.hypertrace.gateway.service.v1.common.OrderByExpression> existingOrderBys =
List<OrderByExpression> existingOrderBys =
super.getRequestOrderByExpressions(request);
// Create an OrderBy Expression based on the interval start time column name. We will need to
// sort based on this
// as the first column.
org.hypertrace.gateway.service.v1.common.OrderByExpression intervalStartTimeOrderBy =
org.hypertrace.gateway.service.v1.common.OrderByExpression.newBuilder()
.setOrder(org.hypertrace.gateway.service.v1.common.SortOrder.ASC)
.setExpression(
org.hypertrace.gateway.service.v1.common.Expression.newBuilder()
.setColumnIdentifier(
org.hypertrace.gateway.service.v1.common.ColumnIdentifier.newBuilder()
.setColumnName(ColumnName.INTERVAL_START_TIME.name())))
.build();

List<org.hypertrace.gateway.service.v1.common.OrderByExpression> orderByExpressions =
List<OrderByExpression> resolvedOrderBys =
new ArrayList<>();
// Add the intervalStartTime OrderBy first and then the request's actual order by expressions.
orderByExpressions.add(intervalStartTimeOrderBy);
orderByExpressions.addAll(existingOrderBys);

return orderByExpressions;
if (!this.containsIntervalOrdering(existingOrderBys)) {
// Create an OrderBy Expression based on the interval start time column name. We will need to
// sort based on this as the first column.
OrderByExpression defaultIntervalOrdering =
OrderByExpression.newBuilder()
.setOrder(SortOrder.ASC)
.setExpression(
Expression.newBuilder()
.setColumnIdentifier(
ColumnIdentifier.newBuilder()
.setColumnName(ColumnName.INTERVAL_START_TIME.name())))
.build();

resolvedOrderBys.add(defaultIntervalOrdering);
}

resolvedOrderBys.addAll(existingOrderBys);

return resolvedOrderBys;
}

private boolean containsIntervalOrdering(List<OrderByExpression> orderByExpressions) {
return orderByExpressions.stream()
.map(OrderByExpression::getExpression)
.map(Expression::getColumnIdentifier)
.map(ColumnIdentifier::getColumnName)
.anyMatch(name -> name.equals(ColumnName.INTERVAL_START_TIME.name()));
}

private void addTimeAggregationsToRequest(
Expand Down Expand Up @@ -183,7 +198,7 @@ protected void handleQueryServiceResponseSingleRow(
// We will need to manually create a Long type value for it since it's a timestamp.
org.hypertrace.gateway.service.v1.common.Value timeColumnValue =
org.hypertrace.gateway.service.v1.common.Value.newBuilder()
.setValueType(org.hypertrace.gateway.service.v1.common.ValueType.LONG)
.setValueType(ValueType.LONG)
.setLong(Long.parseLong(row.getColumn(0).getString()))
.build();

Expand Down Expand Up @@ -227,7 +242,7 @@ protected void handleQueryServiceResponseSingleColumn(
@Override
protected List<org.hypertrace.gateway.service.v1.common.Row.Builder> sortAndPaginateRowBuilders(
List<org.hypertrace.gateway.service.v1.common.Row.Builder> rowBuilders,
List<org.hypertrace.gateway.service.v1.common.OrderByExpression> orderByExpressions,
List<OrderByExpression> orderByExpressions,
int limit,
int offset) {
RowComparator rowComparator = new RowComparator(orderByExpressions);
Expand Down
48 changes: 48 additions & 0 deletions .../test/java/org/hypertrace/gateway/service/explore/TimeAggregationsRequestHandlerTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -199,4 +199,52 @@ public void intervalStartTimeOrderByShouldBeAddedToOrderByListAndAliasShouldMatc
.build(),
orderByExpressions.get(1));
}

@Test
public void intervalStartTimeOrderingNotAddedIfAlreadyRequested() {
ExploreRequest exploreRequest =
ExploreRequest.newBuilder()
.addTimeAggregation(
TimeAggregation.newBuilder()
.setPeriod(Period.newBuilder().setUnit("SECONDS").setValue(60))
.setAggregation(
Expression.newBuilder()
.setFunction(
FunctionExpression.newBuilder()
.setFunction(FunctionType.MAX)
.setAlias("MAX_Duration")
.addArguments(
Expression.newBuilder()
.setColumnIdentifier(
ColumnIdentifier.newBuilder()
.setColumnName("duration"))))))
.addOrderBy(
OrderByExpression.newBuilder()
.setOrder(SortOrder.DESC)
.setExpression(
Expression.newBuilder()
.setColumnIdentifier(
ColumnIdentifier.newBuilder()
.setColumnName(ColumnName.INTERVAL_START_TIME.name()))))
.build();

TimeAggregationsRequestHandler requestHandler =
new TimeAggregationsRequestHandler(
mock(QueryServiceClient.class), 500, mock(AttributeMetadataProvider.class));
List<OrderByExpression> orderByExpressions =
requestHandler.getRequestOrderByExpressions(exploreRequest);

// Should maintain the interval start time order as only order by
Assertions.assertEquals(
List.of(
OrderByExpression.newBuilder()
.setOrder(SortOrder.DESC)
.setExpression(
Expression.newBuilder()
.setColumnIdentifier(
ColumnIdentifier.newBuilder()
.setColumnName(ColumnName.INTERVAL_START_TIME.name())))
.build()),
orderByExpressions);
}
}
14 changes: 7 additions & 7 deletions gateway-service/build.gradle.kts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,10 @@ plugins {
dependencies {
implementation(project(":gateway-service-impl"))

implementation("org.hypertrace.core.grpcutils:grpc-server-utils:0.3.2")
implementation("org.hypertrace.core.serviceframework:platform-service-framework:0.1.18")
implementation("org.hypertrace.core.grpcutils:grpc-server-utils:0.3.4")
implementation("org.hypertrace.core.serviceframework:platform-service-framework:0.1.21")

implementation("io.grpc:grpc-netty:1.33.1")
implementation("io.grpc:grpc-netty:1.36.0")

// Logging
implementation("org.slf4j:slf4j-api:1.7.30")
Expand All @@ -23,11 +23,11 @@ dependencies {
implementation("com.typesafe:config:1.4.1")

constraints {
runtimeOnly("io.netty:netty-codec-http2:4.1.54.Final") {
because("https://snyk.io/vuln/SNYK-JAVA-IONETTY-1020439")
runtimeOnly("io.netty:netty-codec-http2:4.1.60.Final") {
because("https://snyk.io/vuln/SNYK-JAVA-IONETTY-1083991")
}
runtimeOnly("io.netty:netty-handler-proxy:4.1.54.Final") {
because("https://snyk.io/vuln/SNYK-JAVA-IONETTY-1020439s")
runtimeOnly("io.netty:netty-handler-proxy:4.1.60.Final") {
because("https://snyk.io/vuln/SNYK-JAVA-IONETTY-1083991")
}
}
}
Expand Down

0 comments on commit 3ff26d4

Please sign in to comment.