Skip to content

Commit 53a8290

Browse files
sanket-mundrasanket-mundra
authored
Added validation for regex passed (#168)
* Added validation for regex passed * Added more tests with different filters * Minor updates * Addressed review comments
1 parent a61ccc8 commit 53a8290

5 files changed

Lines changed: 215 additions & 26 deletions

File tree

‎span-processing-config-service-impl/build.gradle.kts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ dependencies {
1111
implementation(projects.validationUtils)
1212
implementation(projects.configProtoConverter)
1313
implementation(libs.protobuf.javautil)
14+
implementation(libs.google.re2j)
1415
implementation(libs.hypertrace.grpcutils.context)
1516
implementation(libs.hypertrace.grpcutils.client)
1617

‎span-processing-config-service-impl/src/main/java/org/hypertrace/span/processing/config/service/validation/SpanProcessingConfigRequestValidator.java‎

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,15 +3,21 @@
33
import static org.hypertrace.config.validation.GrpcValidatorUtils.printMessage;
44
import static org.hypertrace.config.validation.GrpcValidatorUtils.validateNonDefaultPresenceOrThrow;
55
import static org.hypertrace.config.validation.GrpcValidatorUtils.validateRequestContextOrThrow;
6+
import static org.hypertrace.span.processing.config.service.v1.RelationalOperator.RELATIONAL_OPERATOR_REGEX_MATCH;
67
import static org.hypertrace.span.processing.config.service.v1.RuleType.RULE_TYPE_SYSTEM;
78

9+
import com.google.re2j.Pattern;
10+
import com.google.re2j.PatternSyntaxException;
811
import io.grpc.Status;
912
import org.hypertrace.core.grpcutils.context.RequestContext;
1013
import org.hypertrace.span.processing.config.service.v1.CreateExcludeSpanRuleRequest;
1114
import org.hypertrace.span.processing.config.service.v1.DeleteExcludeSpanRuleRequest;
1215
import org.hypertrace.span.processing.config.service.v1.ExcludeSpanRuleInfo;
1316
import org.hypertrace.span.processing.config.service.v1.GetAllExcludeSpanRulesRequest;
17+
import org.hypertrace.span.processing.config.service.v1.LogicalSpanFilterExpression;
18+
import org.hypertrace.span.processing.config.service.v1.RelationalSpanFilterExpression;
1419
import org.hypertrace.span.processing.config.service.v1.SpanFilter;
20+
import org.hypertrace.span.processing.config.service.v1.SpanFilterValue;
1521
import org.hypertrace.span.processing.config.service.v1.UpdateExcludeSpanRule;
1622
import org.hypertrace.span.processing.config.service.v1.UpdateExcludeSpanRuleRequest;
1723

@@ -59,12 +65,44 @@ private void validateUpdateRule(UpdateExcludeSpanRule updateExcludeSpanRule) {
5965
private void validateSpanFilter(SpanFilter filter) {
6066
switch (filter.getSpanFilterExpressionCase()) {
6167
case LOGICAL_SPAN_FILTER:
68+
validateLogicalSpanFilter(filter);
69+
break;
6270
case RELATIONAL_SPAN_FILTER:
71+
validateRelationalSpanFilter(filter);
6372
break;
6473
default:
6574
throw Status.INVALID_ARGUMENT
6675
.withDescription("Unexpected filter case: " + printMessage(filter))
6776
.asRuntimeException();
6877
}
6978
}
79+
80+
private void validateLogicalSpanFilter(SpanFilter filter) {
81+
validateNonDefaultPresenceOrThrow(
82+
filter.getLogicalSpanFilter(), LogicalSpanFilterExpression.OPERATOR_FIELD_NUMBER);
83+
validateNonDefaultPresenceOrThrow(
84+
filter.getLogicalSpanFilter(), LogicalSpanFilterExpression.OPERANDS_FIELD_NUMBER);
85+
filter.getLogicalSpanFilter().getOperandsList().forEach(this::validateSpanFilter);
86+
}
87+
88+
private void validateRelationalSpanFilter(SpanFilter filter) {
89+
validateNonDefaultPresenceOrThrow(
90+
filter.getRelationalSpanFilter(), RelationalSpanFilterExpression.OPERATOR_FIELD_NUMBER);
91+
92+
final SpanFilterValue rhs = filter.getRelationalSpanFilter().getRightOperand();
93+
if (filter.getRelationalSpanFilter().getOperator().equals(RELATIONAL_OPERATOR_REGEX_MATCH)) {
94+
validateNonDefaultPresenceOrThrow(rhs, SpanFilterValue.STRING_VALUE_FIELD_NUMBER);
95+
validateRegex(rhs.getStringValue());
96+
}
97+
}
98+
99+
private void validateRegex(String regex) {
100+
try {
101+
Pattern.compile(regex);
102+
} catch (PatternSyntaxException e) {
103+
throw Status.INVALID_ARGUMENT
104+
.withDescription("Invalid Regex pattern: " + regex)
105+
.asRuntimeException();
106+
}
107+
}
70108
}

‎span-processing-config-service-impl/src/test/java/org/hypertrace/span/processing/config/service/validation/SpanProcessingRequestValidatorTest.java‎

Lines changed: 149 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010

1111
import io.grpc.Status;
1212
import io.grpc.StatusRuntimeException;
13+
import java.util.List;
1314
import java.util.Objects;
1415
import java.util.Optional;
1516
import org.hypertrace.core.grpcutils.context.RequestContext;
@@ -18,6 +19,8 @@
1819
import org.hypertrace.span.processing.config.service.v1.ExcludeSpanRuleInfo;
1920
import org.hypertrace.span.processing.config.service.v1.Field;
2021
import org.hypertrace.span.processing.config.service.v1.GetAllExcludeSpanRulesRequest;
22+
import org.hypertrace.span.processing.config.service.v1.LogicalOperator;
23+
import org.hypertrace.span.processing.config.service.v1.LogicalSpanFilterExpression;
2124
import org.hypertrace.span.processing.config.service.v1.RelationalOperator;
2225
import org.hypertrace.span.processing.config.service.v1.RelationalSpanFilterExpression;
2326
import org.hypertrace.span.processing.config.service.v1.SpanFilter;
@@ -107,6 +110,36 @@ void validatesExcludeSpanRuleCreateRequest() {
107110
.build())
108111
.build()));
109112

113+
assertInvalidArgStatusContaining(
114+
"Invalid Regex pattern",
115+
() ->
116+
validator.validateOrThrow(
117+
mockRequestContext,
118+
CreateExcludeSpanRuleRequest.newBuilder()
119+
.setRuleInfo(
120+
ExcludeSpanRuleInfo.newBuilder()
121+
.setName("name")
122+
.setDisabled(true)
123+
.setFilter(buildInvalidRegexTestFilterWithAndOperator())
124+
.setType(RULE_TYPE_USER)
125+
.build())
126+
.build()));
127+
128+
assertInvalidArgStatusContaining(
129+
"Invalid Regex pattern",
130+
() ->
131+
validator.validateOrThrow(
132+
mockRequestContext,
133+
CreateExcludeSpanRuleRequest.newBuilder()
134+
.setRuleInfo(
135+
ExcludeSpanRuleInfo.newBuilder()
136+
.setName("name")
137+
.setDisabled(true)
138+
.setFilter(buildInvalidRegexTestFilterWithOrOperator())
139+
.setType(RULE_TYPE_USER)
140+
.build())
141+
.build()));
142+
110143
assertDoesNotThrow(
111144
() ->
112145
validator.validateOrThrow(
@@ -120,6 +153,35 @@ void validatesExcludeSpanRuleCreateRequest() {
120153
.setType(RULE_TYPE_USER)
121154
.build())
122155
.build()));
156+
157+
assertInvalidArgStatusContaining(
158+
"LogicalSpanFilterExpression.operands",
159+
() ->
160+
validator.validateOrThrow(
161+
mockRequestContext,
162+
CreateExcludeSpanRuleRequest.newBuilder()
163+
.setRuleInfo(
164+
ExcludeSpanRuleInfo.newBuilder()
165+
.setName("name")
166+
.setDisabled(true)
167+
.setFilter(buildTestLogicalFilterWithNoOperands())
168+
.setType(RULE_TYPE_USER)
169+
.build())
170+
.build()));
171+
172+
assertDoesNotThrow(
173+
() ->
174+
validator.validateOrThrow(
175+
mockRequestContext,
176+
CreateExcludeSpanRuleRequest.newBuilder()
177+
.setRuleInfo(
178+
ExcludeSpanRuleInfo.newBuilder()
179+
.setName("name")
180+
.setDisabled(true)
181+
.setFilter(buildRegexTestFilterWithOrOperator())
182+
.setType(RULE_TYPE_USER)
183+
.build())
184+
.build()));
123185
}
124186

125187
@Test
@@ -182,4 +244,91 @@ private SpanFilter buildTestFilter() {
182244
.build())
183245
.build();
184246
}
247+
248+
private SpanFilter buildInvalidRegexTestFilterWithAndOperator() {
249+
return SpanFilter.newBuilder()
250+
.setLogicalSpanFilter(
251+
LogicalSpanFilterExpression.newBuilder()
252+
.setOperator(LogicalOperator.LOGICAL_OPERATOR_AND)
253+
.addAllOperands(
254+
List.of(
255+
SpanFilter.newBuilder()
256+
.setRelationalSpanFilter(
257+
RelationalSpanFilterExpression.newBuilder()
258+
.setField(Field.FIELD_SERVICE_NAME)
259+
.setOperator(RelationalOperator.RELATIONAL_OPERATOR_CONTAINS)
260+
.setRightOperand(
261+
SpanFilterValue.newBuilder().setStringValue("a")))
262+
.build(),
263+
SpanFilter.newBuilder()
264+
.setRelationalSpanFilter(
265+
RelationalSpanFilterExpression.newBuilder()
266+
.setField(Field.FIELD_SERVICE_NAME)
267+
.setOperator(RelationalOperator.RELATIONAL_OPERATOR_REGEX_MATCH)
268+
.setRightOperand(
269+
SpanFilterValue.newBuilder().setStringValue("[(test")))
270+
.build())))
271+
.build();
272+
}
273+
274+
private SpanFilter buildInvalidRegexTestFilterWithOrOperator() {
275+
return SpanFilter.newBuilder()
276+
.setLogicalSpanFilter(
277+
LogicalSpanFilterExpression.newBuilder()
278+
.setOperator(LogicalOperator.LOGICAL_OPERATOR_OR)
279+
.addAllOperands(
280+
List.of(
281+
SpanFilter.newBuilder()
282+
.setRelationalSpanFilter(
283+
RelationalSpanFilterExpression.newBuilder()
284+
.setField(Field.FIELD_SERVICE_NAME)
285+
.setOperator(RelationalOperator.RELATIONAL_OPERATOR_CONTAINS)
286+
.setRightOperand(
287+
SpanFilterValue.newBuilder().setStringValue("a")))
288+
.build(),
289+
SpanFilter.newBuilder()
290+
.setRelationalSpanFilter(
291+
RelationalSpanFilterExpression.newBuilder()
292+
.setField(Field.FIELD_SERVICE_NAME)
293+
.setOperator(RelationalOperator.RELATIONAL_OPERATOR_REGEX_MATCH)
294+
.setRightOperand(
295+
SpanFilterValue.newBuilder().setStringValue("[(test")))
296+
.build())))
297+
.build();
298+
}
299+
300+
private SpanFilter buildRegexTestFilterWithOrOperator() {
301+
return SpanFilter.newBuilder()
302+
.setLogicalSpanFilter(
303+
LogicalSpanFilterExpression.newBuilder()
304+
.setOperator(LogicalOperator.LOGICAL_OPERATOR_OR)
305+
.addAllOperands(
306+
List.of(
307+
SpanFilter.newBuilder()
308+
.setRelationalSpanFilter(
309+
RelationalSpanFilterExpression.newBuilder()
310+
.setField(Field.FIELD_SERVICE_NAME)
311+
.setOperator(RelationalOperator.RELATIONAL_OPERATOR_CONTAINS)
312+
.setRightOperand(
313+
SpanFilterValue.newBuilder().setStringValue("a")))
314+
.build(),
315+
SpanFilter.newBuilder()
316+
.setRelationalSpanFilter(
317+
RelationalSpanFilterExpression.newBuilder()
318+
.setField(Field.FIELD_SERVICE_NAME)
319+
.setOperator(RelationalOperator.RELATIONAL_OPERATOR_REGEX_MATCH)
320+
.setRightOperand(
321+
SpanFilterValue.newBuilder().setStringValue(".*test")))
322+
.build())))
323+
.build();
324+
}
325+
326+
private SpanFilter buildTestLogicalFilterWithNoOperands() {
327+
return SpanFilter.newBuilder()
328+
.setLogicalSpanFilter(
329+
LogicalSpanFilterExpression.newBuilder()
330+
.setOperator(LogicalOperator.LOGICAL_OPERATOR_OR)
331+
.addAllOperands(List.of()))
332+
.build();
333+
}
185334
}

‎span-processing-utils/src/main/java/org/hypertrace/config/span/processing/utils/SpanFilterMatcher.java‎

Lines changed: 21 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -99,33 +99,28 @@ public boolean matches(String lhs, SpanFilterValue rhs, RelationalOperator relat
9999
}
100100

101101
private boolean matches(String lhs, String rhs, RelationalOperator relationalOperator) {
102-
switch (relationalOperator) {
103-
case RELATIONAL_OPERATOR_CONTAINS:
104-
return lhs.contains(rhs);
105-
case RELATIONAL_OPERATOR_EQUALS:
106-
return lhs.equals(rhs);
107-
case RELATIONAL_OPERATOR_NOT_EQUALS:
108-
return !lhs.equals(rhs);
109-
case RELATIONAL_OPERATOR_STARTS_WITH:
110-
return lhs.startsWith(rhs);
111-
case RELATIONAL_OPERATOR_ENDS_WITH:
112-
return lhs.endsWith(rhs);
113-
case RELATIONAL_OPERATOR_REGEX_MATCH:
114-
try {
102+
try {
103+
switch (relationalOperator) {
104+
case RELATIONAL_OPERATOR_CONTAINS:
105+
return lhs.contains(rhs);
106+
case RELATIONAL_OPERATOR_EQUALS:
107+
return lhs.equals(rhs);
108+
case RELATIONAL_OPERATOR_NOT_EQUALS:
109+
return !lhs.equals(rhs);
110+
case RELATIONAL_OPERATOR_STARTS_WITH:
111+
return lhs.startsWith(rhs);
112+
case RELATIONAL_OPERATOR_ENDS_WITH:
113+
return lhs.endsWith(rhs);
114+
case RELATIONAL_OPERATOR_REGEX_MATCH:
115115
return Pattern.compile(rhs).matcher(lhs).find();
116-
} catch (Exception e) {
117-
log.error("Invalid regex: {} passed to match", rhs);
118-
if (log.isDebugEnabled()) {
119-
log.debug(
120-
"Invalid regex passed to match. Hence returning false. lhs: {} and rhs: {}",
121-
lhs,
122-
rhs);
123-
}
124-
return false;
125-
}
126-
default:
127-
log.error("Unsupported relational operator for string value rhs:{}", relationalOperator);
128-
return false;
116+
default:
117+
throw new IllegalStateException(
118+
"Unsupported relational operator for string value rhs: {}" + relationalOperator);
119+
}
120+
} catch (Exception e) {
121+
log.error(
122+
"Unable to match lhs: {} with rhs: {} for operator: {}", lhs, rhs, relationalOperator);
123+
return false;
129124
}
130125
}
131126

‎span-processing-utils/src/test/java/org/hypertrace/config/span/processing/utils/SpanFilterMatcherTest.java‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -118,6 +118,12 @@ void testMatcher() {
118118
"name",
119119
buildSpanFilterValue(List.of("name", "name1")),
120120
RelationalOperator.RELATIONAL_OPERATOR_CONTAINS));
121+
122+
assertFalse(
123+
this.spanFilterMatcher.matches(
124+
"name",
125+
buildSpanFilterValue("[(name"),
126+
RelationalOperator.RELATIONAL_OPERATOR_REGEX_MATCH));
121127
}
122128

123129
@Test

0 commit comments

Comments
 (0)