diff --git a/span-processing-config-service-impl/build.gradle.kts b/span-processing-config-service-impl/build.gradle.kts index 658e40df..2375e397 100644 --- a/span-processing-config-service-impl/build.gradle.kts +++ b/span-processing-config-service-impl/build.gradle.kts @@ -11,6 +11,7 @@ dependencies { implementation(projects.validationUtils) implementation(projects.configProtoConverter) implementation(libs.protobuf.javautil) + implementation(libs.google.re2j) implementation(libs.hypertrace.grpcutils.context) implementation(libs.hypertrace.grpcutils.client) diff --git a/span-processing-config-service-impl/src/main/java/org/hypertrace/span/processing/config/service/validation/SpanProcessingConfigRequestValidator.java b/span-processing-config-service-impl/src/main/java/org/hypertrace/span/processing/config/service/validation/SpanProcessingConfigRequestValidator.java index ee481d52..23e7aafc 100644 --- a/span-processing-config-service-impl/src/main/java/org/hypertrace/span/processing/config/service/validation/SpanProcessingConfigRequestValidator.java +++ b/span-processing-config-service-impl/src/main/java/org/hypertrace/span/processing/config/service/validation/SpanProcessingConfigRequestValidator.java @@ -3,15 +3,21 @@ import static org.hypertrace.config.validation.GrpcValidatorUtils.printMessage; import static org.hypertrace.config.validation.GrpcValidatorUtils.validateNonDefaultPresenceOrThrow; import static org.hypertrace.config.validation.GrpcValidatorUtils.validateRequestContextOrThrow; +import static org.hypertrace.span.processing.config.service.v1.RelationalOperator.RELATIONAL_OPERATOR_REGEX_MATCH; import static org.hypertrace.span.processing.config.service.v1.RuleType.RULE_TYPE_SYSTEM; +import com.google.re2j.Pattern; +import com.google.re2j.PatternSyntaxException; import io.grpc.Status; import org.hypertrace.core.grpcutils.context.RequestContext; import org.hypertrace.span.processing.config.service.v1.CreateExcludeSpanRuleRequest; import org.hypertrace.span.processing.config.service.v1.DeleteExcludeSpanRuleRequest; import org.hypertrace.span.processing.config.service.v1.ExcludeSpanRuleInfo; import org.hypertrace.span.processing.config.service.v1.GetAllExcludeSpanRulesRequest; +import org.hypertrace.span.processing.config.service.v1.LogicalSpanFilterExpression; +import org.hypertrace.span.processing.config.service.v1.RelationalSpanFilterExpression; import org.hypertrace.span.processing.config.service.v1.SpanFilter; +import org.hypertrace.span.processing.config.service.v1.SpanFilterValue; import org.hypertrace.span.processing.config.service.v1.UpdateExcludeSpanRule; import org.hypertrace.span.processing.config.service.v1.UpdateExcludeSpanRuleRequest; @@ -59,7 +65,10 @@ private void validateUpdateRule(UpdateExcludeSpanRule updateExcludeSpanRule) { private void validateSpanFilter(SpanFilter filter) { switch (filter.getSpanFilterExpressionCase()) { case LOGICAL_SPAN_FILTER: + validateLogicalSpanFilter(filter); + break; case RELATIONAL_SPAN_FILTER: + validateRelationalSpanFilter(filter); break; default: throw Status.INVALID_ARGUMENT @@ -67,4 +76,33 @@ private void validateSpanFilter(SpanFilter filter) { .asRuntimeException(); } } + + private void validateLogicalSpanFilter(SpanFilter filter) { + validateNonDefaultPresenceOrThrow( + filter.getLogicalSpanFilter(), LogicalSpanFilterExpression.OPERATOR_FIELD_NUMBER); + validateNonDefaultPresenceOrThrow( + filter.getLogicalSpanFilter(), LogicalSpanFilterExpression.OPERANDS_FIELD_NUMBER); + filter.getLogicalSpanFilter().getOperandsList().forEach(this::validateSpanFilter); + } + + private void validateRelationalSpanFilter(SpanFilter filter) { + validateNonDefaultPresenceOrThrow( + filter.getRelationalSpanFilter(), RelationalSpanFilterExpression.OPERATOR_FIELD_NUMBER); + + final SpanFilterValue rhs = filter.getRelationalSpanFilter().getRightOperand(); + if (filter.getRelationalSpanFilter().getOperator().equals(RELATIONAL_OPERATOR_REGEX_MATCH)) { + validateNonDefaultPresenceOrThrow(rhs, SpanFilterValue.STRING_VALUE_FIELD_NUMBER); + validateRegex(rhs.getStringValue()); + } + } + + private void validateRegex(String regex) { + try { + Pattern.compile(regex); + } catch (PatternSyntaxException e) { + throw Status.INVALID_ARGUMENT + .withDescription("Invalid Regex pattern: " + regex) + .asRuntimeException(); + } + } } diff --git a/span-processing-config-service-impl/src/test/java/org/hypertrace/span/processing/config/service/validation/SpanProcessingRequestValidatorTest.java b/span-processing-config-service-impl/src/test/java/org/hypertrace/span/processing/config/service/validation/SpanProcessingRequestValidatorTest.java index 264984a4..21793693 100644 --- a/span-processing-config-service-impl/src/test/java/org/hypertrace/span/processing/config/service/validation/SpanProcessingRequestValidatorTest.java +++ b/span-processing-config-service-impl/src/test/java/org/hypertrace/span/processing/config/service/validation/SpanProcessingRequestValidatorTest.java @@ -10,6 +10,7 @@ import io.grpc.Status; import io.grpc.StatusRuntimeException; +import java.util.List; import java.util.Objects; import java.util.Optional; import org.hypertrace.core.grpcutils.context.RequestContext; @@ -18,6 +19,8 @@ import org.hypertrace.span.processing.config.service.v1.ExcludeSpanRuleInfo; import org.hypertrace.span.processing.config.service.v1.Field; import org.hypertrace.span.processing.config.service.v1.GetAllExcludeSpanRulesRequest; +import org.hypertrace.span.processing.config.service.v1.LogicalOperator; +import org.hypertrace.span.processing.config.service.v1.LogicalSpanFilterExpression; import org.hypertrace.span.processing.config.service.v1.RelationalOperator; import org.hypertrace.span.processing.config.service.v1.RelationalSpanFilterExpression; import org.hypertrace.span.processing.config.service.v1.SpanFilter; @@ -107,6 +110,36 @@ void validatesExcludeSpanRuleCreateRequest() { .build()) .build())); + assertInvalidArgStatusContaining( + "Invalid Regex pattern", + () -> + validator.validateOrThrow( + mockRequestContext, + CreateExcludeSpanRuleRequest.newBuilder() + .setRuleInfo( + ExcludeSpanRuleInfo.newBuilder() + .setName("name") + .setDisabled(true) + .setFilter(buildInvalidRegexTestFilterWithAndOperator()) + .setType(RULE_TYPE_USER) + .build()) + .build())); + + assertInvalidArgStatusContaining( + "Invalid Regex pattern", + () -> + validator.validateOrThrow( + mockRequestContext, + CreateExcludeSpanRuleRequest.newBuilder() + .setRuleInfo( + ExcludeSpanRuleInfo.newBuilder() + .setName("name") + .setDisabled(true) + .setFilter(buildInvalidRegexTestFilterWithOrOperator()) + .setType(RULE_TYPE_USER) + .build()) + .build())); + assertDoesNotThrow( () -> validator.validateOrThrow( @@ -120,6 +153,35 @@ void validatesExcludeSpanRuleCreateRequest() { .setType(RULE_TYPE_USER) .build()) .build())); + + assertInvalidArgStatusContaining( + "LogicalSpanFilterExpression.operands", + () -> + validator.validateOrThrow( + mockRequestContext, + CreateExcludeSpanRuleRequest.newBuilder() + .setRuleInfo( + ExcludeSpanRuleInfo.newBuilder() + .setName("name") + .setDisabled(true) + .setFilter(buildTestLogicalFilterWithNoOperands()) + .setType(RULE_TYPE_USER) + .build()) + .build())); + + assertDoesNotThrow( + () -> + validator.validateOrThrow( + mockRequestContext, + CreateExcludeSpanRuleRequest.newBuilder() + .setRuleInfo( + ExcludeSpanRuleInfo.newBuilder() + .setName("name") + .setDisabled(true) + .setFilter(buildRegexTestFilterWithOrOperator()) + .setType(RULE_TYPE_USER) + .build()) + .build())); } @Test @@ -182,4 +244,91 @@ private SpanFilter buildTestFilter() { .build()) .build(); } + + private SpanFilter buildInvalidRegexTestFilterWithAndOperator() { + return SpanFilter.newBuilder() + .setLogicalSpanFilter( + LogicalSpanFilterExpression.newBuilder() + .setOperator(LogicalOperator.LOGICAL_OPERATOR_AND) + .addAllOperands( + List.of( + SpanFilter.newBuilder() + .setRelationalSpanFilter( + RelationalSpanFilterExpression.newBuilder() + .setField(Field.FIELD_SERVICE_NAME) + .setOperator(RelationalOperator.RELATIONAL_OPERATOR_CONTAINS) + .setRightOperand( + SpanFilterValue.newBuilder().setStringValue("a"))) + .build(), + SpanFilter.newBuilder() + .setRelationalSpanFilter( + RelationalSpanFilterExpression.newBuilder() + .setField(Field.FIELD_SERVICE_NAME) + .setOperator(RelationalOperator.RELATIONAL_OPERATOR_REGEX_MATCH) + .setRightOperand( + SpanFilterValue.newBuilder().setStringValue("[(test"))) + .build()))) + .build(); + } + + private SpanFilter buildInvalidRegexTestFilterWithOrOperator() { + return SpanFilter.newBuilder() + .setLogicalSpanFilter( + LogicalSpanFilterExpression.newBuilder() + .setOperator(LogicalOperator.LOGICAL_OPERATOR_OR) + .addAllOperands( + List.of( + SpanFilter.newBuilder() + .setRelationalSpanFilter( + RelationalSpanFilterExpression.newBuilder() + .setField(Field.FIELD_SERVICE_NAME) + .setOperator(RelationalOperator.RELATIONAL_OPERATOR_CONTAINS) + .setRightOperand( + SpanFilterValue.newBuilder().setStringValue("a"))) + .build(), + SpanFilter.newBuilder() + .setRelationalSpanFilter( + RelationalSpanFilterExpression.newBuilder() + .setField(Field.FIELD_SERVICE_NAME) + .setOperator(RelationalOperator.RELATIONAL_OPERATOR_REGEX_MATCH) + .setRightOperand( + SpanFilterValue.newBuilder().setStringValue("[(test"))) + .build()))) + .build(); + } + + private SpanFilter buildRegexTestFilterWithOrOperator() { + return SpanFilter.newBuilder() + .setLogicalSpanFilter( + LogicalSpanFilterExpression.newBuilder() + .setOperator(LogicalOperator.LOGICAL_OPERATOR_OR) + .addAllOperands( + List.of( + SpanFilter.newBuilder() + .setRelationalSpanFilter( + RelationalSpanFilterExpression.newBuilder() + .setField(Field.FIELD_SERVICE_NAME) + .setOperator(RelationalOperator.RELATIONAL_OPERATOR_CONTAINS) + .setRightOperand( + SpanFilterValue.newBuilder().setStringValue("a"))) + .build(), + SpanFilter.newBuilder() + .setRelationalSpanFilter( + RelationalSpanFilterExpression.newBuilder() + .setField(Field.FIELD_SERVICE_NAME) + .setOperator(RelationalOperator.RELATIONAL_OPERATOR_REGEX_MATCH) + .setRightOperand( + SpanFilterValue.newBuilder().setStringValue(".*test"))) + .build()))) + .build(); + } + + private SpanFilter buildTestLogicalFilterWithNoOperands() { + return SpanFilter.newBuilder() + .setLogicalSpanFilter( + LogicalSpanFilterExpression.newBuilder() + .setOperator(LogicalOperator.LOGICAL_OPERATOR_OR) + .addAllOperands(List.of())) + .build(); + } } diff --git a/span-processing-utils/src/main/java/org/hypertrace/config/span/processing/utils/SpanFilterMatcher.java b/span-processing-utils/src/main/java/org/hypertrace/config/span/processing/utils/SpanFilterMatcher.java index d069ab34..ea9092e8 100644 --- a/span-processing-utils/src/main/java/org/hypertrace/config/span/processing/utils/SpanFilterMatcher.java +++ b/span-processing-utils/src/main/java/org/hypertrace/config/span/processing/utils/SpanFilterMatcher.java @@ -99,33 +99,28 @@ public boolean matches(String lhs, SpanFilterValue rhs, RelationalOperator relat } private boolean matches(String lhs, String rhs, RelationalOperator relationalOperator) { - switch (relationalOperator) { - case RELATIONAL_OPERATOR_CONTAINS: - return lhs.contains(rhs); - case RELATIONAL_OPERATOR_EQUALS: - return lhs.equals(rhs); - case RELATIONAL_OPERATOR_NOT_EQUALS: - return !lhs.equals(rhs); - case RELATIONAL_OPERATOR_STARTS_WITH: - return lhs.startsWith(rhs); - case RELATIONAL_OPERATOR_ENDS_WITH: - return lhs.endsWith(rhs); - case RELATIONAL_OPERATOR_REGEX_MATCH: - try { + try { + switch (relationalOperator) { + case RELATIONAL_OPERATOR_CONTAINS: + return lhs.contains(rhs); + case RELATIONAL_OPERATOR_EQUALS: + return lhs.equals(rhs); + case RELATIONAL_OPERATOR_NOT_EQUALS: + return !lhs.equals(rhs); + case RELATIONAL_OPERATOR_STARTS_WITH: + return lhs.startsWith(rhs); + case RELATIONAL_OPERATOR_ENDS_WITH: + return lhs.endsWith(rhs); + case RELATIONAL_OPERATOR_REGEX_MATCH: return Pattern.compile(rhs).matcher(lhs).find(); - } catch (Exception e) { - log.error("Invalid regex: {} passed to match", rhs); - if (log.isDebugEnabled()) { - log.debug( - "Invalid regex passed to match. Hence returning false. lhs: {} and rhs: {}", - lhs, - rhs); - } - return false; - } - default: - log.error("Unsupported relational operator for string value rhs:{}", relationalOperator); - return false; + default: + throw new IllegalStateException( + "Unsupported relational operator for string value rhs: {}" + relationalOperator); + } + } catch (Exception e) { + log.error( + "Unable to match lhs: {} with rhs: {} for operator: {}", lhs, rhs, relationalOperator); + return false; } } diff --git a/span-processing-utils/src/test/java/org/hypertrace/config/span/processing/utils/SpanFilterMatcherTest.java b/span-processing-utils/src/test/java/org/hypertrace/config/span/processing/utils/SpanFilterMatcherTest.java index 5b2af29e..52cf47c7 100644 --- a/span-processing-utils/src/test/java/org/hypertrace/config/span/processing/utils/SpanFilterMatcherTest.java +++ b/span-processing-utils/src/test/java/org/hypertrace/config/span/processing/utils/SpanFilterMatcherTest.java @@ -118,6 +118,12 @@ void testMatcher() { "name", buildSpanFilterValue(List.of("name", "name1")), RelationalOperator.RELATIONAL_OPERATOR_CONTAINS)); + + assertFalse( + this.spanFilterMatcher.matches( + "name", + buildSpanFilterValue("[(name"), + RelationalOperator.RELATIONAL_OPERATOR_REGEX_MATCH)); } @Test