Skip to content

Commit c4f281b

Browse files
pavolloffaypavolloffay
authored
Change filter API to return primitive boolean (#142)
* Change filter API to return primitive boolean Signed-off-by: Pavol Loffay <p.loffay@gmail.com> * Fix Signed-off-by: Pavol Loffay <p.loffay@gmail.com>
1 parent cf13a6a commit c4f281b

File tree

11 files changed

+46
-150
lines changed

11 files changed

+46
-150
lines changed

‎filter-api/src/main/java/org/hypertrace/agent/filter/MultiFilter.java‎

Lines changed: 6 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -19,10 +19,7 @@
1919
import io.opentelemetry.api.trace.Span;
2020
import java.util.List;
2121
import java.util.Map;
22-
import org.hypertrace.agent.filter.api.ExecutionBlocked;
23-
import org.hypertrace.agent.filter.api.ExecutionNotBlocked;
2422
import org.hypertrace.agent.filter.api.Filter;
25-
import org.hypertrace.agent.filter.api.FilterResult;
2623

2724
class MultiFilter implements Filter {
2825

@@ -33,24 +30,24 @@ public MultiFilter(List<Filter> filters) {
3330
}
3431

3532
@Override
36-
public FilterResult evaluateRequestHeaders(Span span, Map<String, String> headers) {
33+
public boolean evaluateRequestHeaders(Span span, Map<String, String> headers) {
3734
boolean shouldBlock = false;
3835
for (Filter filter : filters) {
39-
if (filter.evaluateRequestHeaders(span, headers).blockExecution()) {
36+
if (filter.evaluateRequestHeaders(span, headers)) {
4037
shouldBlock = true;
4138
}
4239
}
43-
return shouldBlock ? ExecutionBlocked.INSTANCE : ExecutionNotBlocked.INSTANCE;
40+
return shouldBlock;
4441
}
4542

4643
@Override
47-
public FilterResult evaluateRequestBody(Span span, String body) {
44+
public boolean evaluateRequestBody(Span span, String body) {
4845
boolean shouldBlock = false;
4946
for (Filter filter : filters) {
50-
if (filter.evaluateRequestBody(span, body).blockExecution()) {
47+
if (filter.evaluateRequestBody(span, body)) {
5148
shouldBlock = true;
5249
}
5350
}
54-
return shouldBlock ? ExecutionBlocked.INSTANCE : ExecutionNotBlocked.INSTANCE;
51+
return shouldBlock;
5552
}
5653
}

‎filter-api/src/main/java/org/hypertrace/agent/filter/api/ExecutionBlocked.java‎

Lines changed: 0 additions & 30 deletions
This file was deleted.

‎filter-api/src/main/java/org/hypertrace/agent/filter/api/ExecutionNotBlocked.java‎

Lines changed: 0 additions & 29 deletions
This file was deleted.

‎filter-api/src/main/java/org/hypertrace/agent/filter/api/Filter.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -32,13 +32,13 @@ public interface Filter {
3232
* @param headers are used for blocking evaluation.
3333
* @return filter result
3434
*/
35-
FilterResult evaluateRequestHeaders(Span span, Map<String, String> headers);
35+
boolean evaluateRequestHeaders(Span span, Map<String, String> headers);
3636

3737
/**
3838
* Evaluate the execution.
3939
*
4040
* @param body request body
4141
* @return filter result
4242
*/
43-
FilterResult evaluateRequestBody(Span span, String body);
43+
boolean evaluateRequestBody(Span span, String body);
4444
}

‎filter-api/src/main/java/org/hypertrace/agent/filter/api/FilterResult.java‎

Lines changed: 0 additions & 28 deletions
This file was deleted.

‎filter-api/src/main/java/org/hypertrace/agent/filter/mock/MockFilter.java‎

Lines changed: 5 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -18,27 +18,24 @@
1818

1919
import io.opentelemetry.api.trace.Span;
2020
import java.util.Map;
21-
import org.hypertrace.agent.filter.api.ExecutionBlocked;
22-
import org.hypertrace.agent.filter.api.ExecutionNotBlocked;
2321
import org.hypertrace.agent.filter.api.Filter;
24-
import org.hypertrace.agent.filter.api.FilterResult;
2522

2623
/** Mock filter, blocks execution if an attribute with "mockblock" key is present. */
2724
class MockFilter implements Filter {
2825

2926
MockFilter() {}
3027

3128
@Override
32-
public FilterResult evaluateRequestHeaders(Span span, Map<String, String> headers) {
29+
public boolean evaluateRequestHeaders(Span span, Map<String, String> headers) {
3330
if (headers.containsKey("mockblock")) {
3431
span.setAttribute("hypertrace.mock.filter.result", "true");
35-
return ExecutionBlocked.INSTANCE;
32+
return true;
3633
}
37-
return ExecutionNotBlocked.INSTANCE;
34+
return false;
3835
}
3936

4037
@Override
41-
public FilterResult evaluateRequestBody(Span span, String body) {
42-
return ExecutionNotBlocked.INSTANCE;
38+
public boolean evaluateRequestBody(Span span, String body) {
39+
return false;
4340
}
4441
}

‎filter-custom-opa/src/main/java/org/hypertrace/agent/filter/opa/custom/CustomOpaLib.java‎

Lines changed: 4 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -27,10 +27,7 @@
2727
import java.util.concurrent.ThreadFactory;
2828
import java.util.concurrent.TimeUnit;
2929
import java.util.concurrent.atomic.AtomicInteger;
30-
import org.hypertrace.agent.filter.api.ExecutionBlocked;
31-
import org.hypertrace.agent.filter.api.ExecutionNotBlocked;
3230
import org.hypertrace.agent.filter.api.Filter;
33-
import org.hypertrace.agent.filter.api.FilterResult;
3431
import org.hypertrace.agent.filter.opa.custom.evaluator.ICustomPolicyEvaluator;
3532
import org.hypertrace.agent.filter.opa.custom.evaluator.IpAddressPolicyEvaluator;
3633
import org.slf4j.Logger;
@@ -90,7 +87,7 @@ public void run() {
9087
// }
9188

9289
@Override
93-
public FilterResult evaluateRequestHeaders(Span span, Map<String, String> headers) {
90+
public boolean evaluateRequestHeaders(Span span, Map<String, String> headers) {
9491
// currently as per policy.rego, allowed list has precedence over denylist
9592
boolean allow =
9693
policyEvaluators.stream()
@@ -99,11 +96,11 @@ public FilterResult evaluateRequestHeaders(Span span, Map<String, String> header
9996
policyEvaluator.allow(opaCommunicator.getBlockingData(), headers))
10097
.anyMatch(Boolean::booleanValue);
10198
span.setAttribute(OPA_RESULT, String.valueOf(allow));
102-
return allow ? ExecutionNotBlocked.INSTANCE : ExecutionBlocked.INSTANCE;
99+
return !allow;
103100
}
104101

105102
@Override
106-
public FilterResult evaluateRequestBody(Span span, String body) {
107-
return ExecutionNotBlocked.INSTANCE;
103+
public boolean evaluateRequestBody(Span span, String body) {
104+
return true;
108105
}
109106
}

‎instrumentation/grpc-1.5/src/main/java/io/opentelemetry/instrumentation/hypertrace/grpc/v1_5/server/GrpcServerInterceptor.java‎

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,6 @@
3131
import org.hypertrace.agent.core.HypertraceConfig;
3232
import org.hypertrace.agent.core.HypertraceSemanticAttributes;
3333
import org.hypertrace.agent.filter.FilterRegistry;
34-
import org.hypertrace.agent.filter.api.FilterResult;
3534

3635
public class GrpcServerInterceptor implements ServerInterceptor {
3736

@@ -52,9 +51,8 @@ public <ReqT, RespT> ServerCall.Listener<ReqT> interceptCall(
5251
mapHeaders, currentSpan, HypertraceSemanticAttributes::rpcRequestMetadata);
5352
}
5453

55-
FilterResult filterResult =
56-
FilterRegistry.getFilter().evaluateRequestHeaders(currentSpan, mapHeaders);
57-
if (filterResult.blockExecution()) {
54+
boolean block = FilterRegistry.getFilter().evaluateRequestHeaders(currentSpan, mapHeaders);
55+
if (block) {
5856
call.close(Status.PERMISSION_DENIED, new Metadata());
5957
@SuppressWarnings("unchecked")
6058
ServerCall.Listener<ReqT> noop = NoopServerCallListener.INSTANCE;

‎instrumentation/servlet/servlet-2.3/src/main/java/io/opentelemetry/instrumentation/hypertrace/servlet/v2_3/Servlet2BodyInstrumentationModule.java‎

Lines changed: 9 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,6 @@
4747
import org.hypertrace.agent.core.HypertraceConfig;
4848
import org.hypertrace.agent.core.HypertraceSemanticAttributes;
4949
import org.hypertrace.agent.filter.FilterRegistry;
50-
import org.hypertrace.agent.filter.api.FilterResult;
5150

5251
/**
5352
* Body capture for servlet 2.3. Note that OTEL servlet instrumentation is compatible with servlet
@@ -124,26 +123,26 @@ public static class Filter2Advice {
124123
// request attribute key injected at first filerChain.doFilter
125124
private static final String ALREADY_LOADED = "__org.hypertrace.agent.on_start_executed";
126125

127-
@Advice.OnMethodEnter(suppress = Throwable.class, skipOn = FilterResult.class)
128-
public static Object start(
126+
@Advice.OnMethodEnter(suppress = Throwable.class, skipOn = Advice.OnNonDefaultValue.class)
127+
public static boolean start(
129128
@Advice.Argument(value = 0, readOnly = false) ServletRequest request,
130129
@Advice.Argument(value = 1, readOnly = false) ServletResponse response,
131130
@Advice.Local("rootStart") Boolean rootStart) {
132131

133132
if (!HypertraceConfig.isInstrumentationEnabled(
134133
Servlet2InstrumentationName.PRIMARY, Servlet2InstrumentationName.OTHER)) {
135-
return null;
134+
return false;
136135
}
137136
if (!(request instanceof HttpServletRequest) || !(response instanceof HttpServletResponse)) {
138-
return null;
137+
return false;
139138
}
140139

141140
// TODO run on every doFilter and check if user removed wrapper
142141
// TODO what if user unwraps request and reads the body?
143142

144143
// run the instrumentation only for the root FilterChain.doFilter()
145144
if (request.getAttribute(ALREADY_LOADED) != null) {
146-
return null;
145+
return false;
147146
}
148147
request.setAttribute(ALREADY_LOADED, true);
149148

@@ -171,13 +170,12 @@ public static Object start(
171170
}
172171
headers.put(headerName, headerValue);
173172
}
174-
FilterResult filterResult =
175-
FilterRegistry.getFilter().evaluateRequestHeaders(currentSpan, headers);
176-
if (filterResult.blockExecution()) {
173+
boolean block = FilterRegistry.getFilter().evaluateRequestHeaders(currentSpan, headers);
174+
if (block) {
177175
httpResponse.setStatus(403);
178-
return filterResult;
176+
return true;
179177
}
180-
return null;
178+
return false;
181179
}
182180

183181
@Advice.OnMethodExit(onThrowable = Throwable.class, suppress = Throwable.class)

‎instrumentation/servlet/servlet-3.0/src/main/java/io/opentelemetry/instrumentation/hypertrace/servlet/v3_0/Servlet30BodyInstrumentationModule.java‎

Lines changed: 9 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,6 @@
4848
import org.hypertrace.agent.core.HypertraceConfig;
4949
import org.hypertrace.agent.core.HypertraceSemanticAttributes;
5050
import org.hypertrace.agent.filter.FilterRegistry;
51-
import org.hypertrace.agent.filter.api.FilterResult;
5251

5352
@AutoService(InstrumentationModule.class)
5453
public class Servlet30BodyInstrumentationModule extends InstrumentationModule {
@@ -120,26 +119,26 @@ public static class FilterAdvice {
120119
private static final String ALREADY_LOADED = "__org.hypertrace.agent.on_start_executed";
121120
private static final String TRACER_NAME = "org.hypertrace.agent.servlet";
122121

123-
@Advice.OnMethodEnter(suppress = Throwable.class, skipOn = FilterResult.class)
124-
public static Object start(
122+
@Advice.OnMethodEnter(suppress = Throwable.class, skipOn = Advice.OnNonDefaultValue.class)
123+
public static boolean start(
125124
@Advice.Argument(value = 0, readOnly = false) ServletRequest request,
126125
@Advice.Argument(value = 1, readOnly = false) ServletResponse response,
127126
@Advice.Local("rootStart") Boolean rootStart) {
128127

129128
if (!HypertraceConfig.isInstrumentationEnabled(
130129
Servlet30InstrumentationName.PRIMARY, Servlet30InstrumentationName.OTHER)) {
131-
return null;
130+
return false;
132131
}
133132
if (!(request instanceof HttpServletRequest) || !(response instanceof HttpServletResponse)) {
134-
return null;
133+
return false;
135134
}
136135

137136
// TODO run on every doFilter and check if user removed wrapper
138137
// TODO what if user unwraps request and reads the body?
139138

140139
// run the instrumentation only for the root FilterChain.doFilter()
141140
if (request.getAttribute(ALREADY_LOADED) != null) {
142-
return null;
141+
return false;
143142
}
144143
request.setAttribute(ALREADY_LOADED, true);
145144

@@ -165,13 +164,12 @@ public static Object start(
165164
}
166165
headers.put(headerName, headerValue);
167166
}
168-
FilterResult filterResult =
169-
FilterRegistry.getFilter().evaluateRequestHeaders(currentSpan, headers);
170-
if (filterResult.blockExecution()) {
167+
boolean block = FilterRegistry.getFilter().evaluateRequestHeaders(currentSpan, headers);
168+
if (block) {
171169
httpResponse.setStatus(403);
172-
return filterResult;
170+
return true;
173171
}
174-
return null;
172+
return false;
175173
}
176174

177175
@Advice.OnMethodExit(onThrowable = Throwable.class, suppress = Throwable.class)

0 commit comments

Comments
 (0)