From 4570eefbdee4a273ee35be26965c7a8b07952a9f Mon Sep 17 00:00:00 2001 From: Shashank Patidar <74622220+shashank11p@users.noreply.github.com> Date: Wed, 3 Jan 2024 19:34:39 +0530 Subject: [PATCH] Custom block code and msg (#398) * TODOs * add FilterResult and custom status code and msg for blocking * add response msg and tests * fix tests * revert grpc changes --- .../hypertrace/agent/filter/MultiFilter.java | 21 +++++----- .../hypertrace/agent/filter/api/Filter.java | 5 ++- .../v1_6/server/GrpcServerInterceptor.java | 8 +++- .../grpc/v1_6/GrpcInstrumentationTest.java | 2 +- .../MicronautInstrumentationTest.java | 2 +- .../v3/MicronautInstrumentationTest.java | 2 +- .../HttpServerBlockingRequestHandler.java | 30 +++++++++---- ...tractNetty40ServerInstrumentationTest.java | 14 +++---- .../HttpServerBlockingRequestHandler.java | 30 +++++++++---- ...tractNetty41ServerInstrumentationTest.java | 14 +++---- .../Servlet30AndFilterInstrumentation.java | 19 +++++++-- .../Servlet30InstrumentationTest.java | 3 ++ ...ServletInputStreamInstrumentationTest.java | 5 ++- .../BufferedReaderInstrumentationTest.java | 5 ++- .../webflux/SpringWebfluxServerTest.java | 2 +- .../vertx/VertxServerInstrumentationTest.java | 2 +- .../agent/core/filter/FilterResult.java | 42 +++++++++++++++++++ .../HypertraceEvaluationException.java | 11 ++++- .../buffer/ByteBufferSpanPair.java | 13 +++--- .../buffer/CharBufferSpanPair.java | 13 +++--- .../agent/testing/mockfilter/MockFilter.java | 16 +++---- 21 files changed, 179 insertions(+), 80 deletions(-) create mode 100644 javaagent-core/src/main/java/org/hypertrace/agent/core/filter/FilterResult.java diff --git a/filter-api/src/main/java/org/hypertrace/agent/filter/MultiFilter.java b/filter-api/src/main/java/org/hypertrace/agent/filter/MultiFilter.java index 223d07e3e..0a6999645 100644 --- a/filter-api/src/main/java/org/hypertrace/agent/filter/MultiFilter.java +++ b/filter-api/src/main/java/org/hypertrace/agent/filter/MultiFilter.java @@ -19,6 +19,7 @@ import io.opentelemetry.api.trace.Span; import java.util.List; import java.util.Map; +import org.hypertrace.agent.core.filter.FilterResult; import org.hypertrace.agent.filter.api.Filter; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -34,12 +35,12 @@ public MultiFilter(List filters) { } @Override - public boolean evaluateRequestHeaders(Span span, Map headers) { - boolean shouldBlock = false; + public FilterResult evaluateRequestHeaders(Span span, Map headers) { for (Filter filter : filters) { try { - if (filter.evaluateRequestHeaders(span, headers)) { - shouldBlock = true; + FilterResult filterResult = filter.evaluateRequestHeaders(span, headers); + if (filterResult.shouldBlock()) { + return filterResult; } } catch (Throwable t) { logger.warn( @@ -48,16 +49,16 @@ public boolean evaluateRequestHeaders(Span span, Map headers) { t); } } - return shouldBlock; + return new FilterResult(false, 0, ""); } @Override - public boolean evaluateRequestBody(Span span, String body, Map headers) { - boolean shouldBlock = false; + public FilterResult evaluateRequestBody(Span span, String body, Map headers) { for (Filter filter : filters) { try { - if (filter.evaluateRequestBody(span, body, headers)) { - shouldBlock = true; + FilterResult filterResult = filter.evaluateRequestBody(span, body, headers); + if (filterResult.shouldBlock()) { + return filterResult; } } catch (Throwable t) { logger.warn( @@ -66,6 +67,6 @@ public boolean evaluateRequestBody(Span span, String body, Map h t); } } - return shouldBlock; + return new FilterResult(false, 0, ""); } } diff --git a/filter-api/src/main/java/org/hypertrace/agent/filter/api/Filter.java b/filter-api/src/main/java/org/hypertrace/agent/filter/api/Filter.java index 74dfaeee7..df22efd1f 100644 --- a/filter-api/src/main/java/org/hypertrace/agent/filter/api/Filter.java +++ b/filter-api/src/main/java/org/hypertrace/agent/filter/api/Filter.java @@ -18,6 +18,7 @@ import io.opentelemetry.api.trace.Span; import java.util.Map; +import org.hypertrace.agent.core.filter.FilterResult; import org.hypertrace.agent.filter.FilterRegistry; /** @@ -32,7 +33,7 @@ public interface Filter { * @param headers are used for blocking evaluation. * @return filter result */ - boolean evaluateRequestHeaders(Span span, Map headers); + FilterResult evaluateRequestHeaders(Span span, Map headers); /** * Evaluate the execution. @@ -42,5 +43,5 @@ public interface Filter { * @param headers of the request associated with this body * @return filter result */ - boolean evaluateRequestBody(Span span, String body, Map headers); + FilterResult evaluateRequestBody(Span span, String body, Map headers); } diff --git a/instrumentation/grpc-1.6/src/main/java/io/opentelemetry/javaagent/instrumentation/hypertrace/grpc/v1_6/server/GrpcServerInterceptor.java b/instrumentation/grpc-1.6/src/main/java/io/opentelemetry/javaagent/instrumentation/hypertrace/grpc/v1_6/server/GrpcServerInterceptor.java index bfdcc559c..da965e949 100644 --- a/instrumentation/grpc-1.6/src/main/java/io/opentelemetry/javaagent/instrumentation/hypertrace/grpc/v1_6/server/GrpcServerInterceptor.java +++ b/instrumentation/grpc-1.6/src/main/java/io/opentelemetry/javaagent/instrumentation/hypertrace/grpc/v1_6/server/GrpcServerInterceptor.java @@ -29,6 +29,7 @@ import io.opentelemetry.javaagent.instrumentation.hypertrace.grpc.v1_6.GrpcSpanDecorator; import java.util.Map; import org.hypertrace.agent.core.config.InstrumentationConfig; +import org.hypertrace.agent.core.filter.FilterResult; import org.hypertrace.agent.core.instrumentation.HypertraceSemanticAttributes; import org.hypertrace.agent.filter.FilterRegistry; import org.slf4j.Logger; @@ -57,8 +58,11 @@ public ServerCall.Listener interceptCall( GrpcSpanDecorator.addMetadataAttributes(mapHeaders, currentSpan); } - boolean block = FilterRegistry.getFilter().evaluateRequestHeaders(currentSpan, mapHeaders); - if (block) { + FilterResult filterResult = + FilterRegistry.getFilter().evaluateRequestHeaders(currentSpan, mapHeaders); + if (filterResult.shouldBlock()) { + // We cannot send custom message in grpc calls + // TODO: map http codes with grpc codes. filterResult.getBlockingStatusCode() call.close(Status.PERMISSION_DENIED, new Metadata()); @SuppressWarnings("unchecked") ServerCall.Listener noop = NoopServerCallListener.INSTANCE; diff --git a/instrumentation/grpc-1.6/src/test/java/io/opentelemetry/javaagent/instrumentation/hypertrace/grpc/v1_6/GrpcInstrumentationTest.java b/instrumentation/grpc-1.6/src/test/java/io/opentelemetry/javaagent/instrumentation/hypertrace/grpc/v1_6/GrpcInstrumentationTest.java index 6ad6cf081..770b1e273 100644 --- a/instrumentation/grpc-1.6/src/test/java/io/opentelemetry/javaagent/instrumentation/hypertrace/grpc/v1_6/GrpcInstrumentationTest.java +++ b/instrumentation/grpc-1.6/src/test/java/io/opentelemetry/javaagent/instrumentation/hypertrace/grpc/v1_6/GrpcInstrumentationTest.java @@ -159,7 +159,7 @@ public void serverRequestBlocking() throws TimeoutException, InterruptedExceptio try { Response response = blockingStub.sayHello(REQUEST); } catch (StatusRuntimeException ex) { - Assertions.assertEquals(Status.PERMISSION_DENIED, ex.getStatus()); + Assertions.assertEquals(Status.PERMISSION_DENIED.getCode(), ex.getStatus().getCode()); } TEST_WRITER.waitForSpans(2); diff --git a/instrumentation/micronaut-1.0/src/test/java/io/opentelemetry/javaagent/instrumentation/hypertrace/micronaut/MicronautInstrumentationTest.java b/instrumentation/micronaut-1.0/src/test/java/io/opentelemetry/javaagent/instrumentation/hypertrace/micronaut/MicronautInstrumentationTest.java index 91e8e03e3..05759de8f 100644 --- a/instrumentation/micronaut-1.0/src/test/java/io/opentelemetry/javaagent/instrumentation/hypertrace/micronaut/MicronautInstrumentationTest.java +++ b/instrumentation/micronaut-1.0/src/test/java/io/opentelemetry/javaagent/instrumentation/hypertrace/micronaut/MicronautInstrumentationTest.java @@ -175,7 +175,7 @@ public void blocking() throws IOException, TimeoutException, InterruptedExceptio try (Response response = httpClient.newCall(request).execute()) { Assertions.assertEquals(403, response.code()); - Assertions.assertTrue(response.body().string().isEmpty()); + Assertions.assertEquals("Hypertrace Blocked Request", response.body().string()); } List> traces = TEST_WRITER.getTraces(); diff --git a/instrumentation/micronaut-3.0/src/test/java/io/opentelemetry/javaagent/instrumentation/hypertrace/micronaut/v3/MicronautInstrumentationTest.java b/instrumentation/micronaut-3.0/src/test/java/io/opentelemetry/javaagent/instrumentation/hypertrace/micronaut/v3/MicronautInstrumentationTest.java index 785795912..e2416f722 100644 --- a/instrumentation/micronaut-3.0/src/test/java/io/opentelemetry/javaagent/instrumentation/hypertrace/micronaut/v3/MicronautInstrumentationTest.java +++ b/instrumentation/micronaut-3.0/src/test/java/io/opentelemetry/javaagent/instrumentation/hypertrace/micronaut/v3/MicronautInstrumentationTest.java @@ -175,7 +175,7 @@ public void blocking() throws IOException, TimeoutException, InterruptedExceptio try (Response response = httpClient.newCall(request).execute()) { Assertions.assertEquals(403, response.code()); - Assertions.assertTrue(response.body().string().isEmpty()); + Assertions.assertEquals("Hypertrace Blocked Request", response.body().string()); } List> traces = TEST_WRITER.getTraces(); diff --git a/instrumentation/netty/netty-4.0/src/main/java/io/opentelemetry/javaagent/instrumentation/hypertrace/netty/v4_0/server/HttpServerBlockingRequestHandler.java b/instrumentation/netty/netty-4.0/src/main/java/io/opentelemetry/javaagent/instrumentation/hypertrace/netty/v4_0/server/HttpServerBlockingRequestHandler.java index 68a0b0010..414b18272 100644 --- a/instrumentation/netty/netty-4.0/src/main/java/io/opentelemetry/javaagent/instrumentation/hypertrace/netty/v4_0/server/HttpServerBlockingRequestHandler.java +++ b/instrumentation/netty/netty-4.0/src/main/java/io/opentelemetry/javaagent/instrumentation/hypertrace/netty/v4_0/server/HttpServerBlockingRequestHandler.java @@ -16,6 +16,7 @@ package io.opentelemetry.javaagent.instrumentation.hypertrace.netty.v4_0.server; +import io.netty.buffer.Unpooled; import io.netty.channel.Channel; import io.netty.channel.ChannelFutureListener; import io.netty.channel.ChannelHandlerContext; @@ -30,7 +31,9 @@ import io.opentelemetry.context.Context; import io.opentelemetry.instrumentation.netty.v4.common.HttpRequestAndChannel; import io.opentelemetry.javaagent.instrumentation.hypertrace.netty.v4_0.AttributeKeys; +import java.nio.charset.StandardCharsets; import java.util.Map; +import org.hypertrace.agent.core.filter.FilterResult; import org.hypertrace.agent.filter.FilterRegistry; public class HttpServerBlockingRequestHandler extends ChannelInboundHandlerAdapter { @@ -52,26 +55,37 @@ public void channelRead(ChannelHandlerContext ctx, Object msg) { if (msg instanceof HttpRequest) { Attribute> headersAttr = channel.attr(AttributeKeys.REQUEST_HEADERS); Map headers = headersAttr.getAndRemove(); - if (headers != null && FilterRegistry.getFilter().evaluateRequestHeaders(span, headers)) { - forbidden(ctx, (HttpRequest) msg); - return; + if (headers != null) { + FilterResult filterResult = + FilterRegistry.getFilter().evaluateRequestHeaders(span, headers); + if (filterResult.shouldBlock()) { + forbidden(ctx, (HttpRequest) msg, filterResult); + return; + } } } if (msg instanceof HttpContent) { - if (FilterRegistry.getFilter().evaluateRequestBody(span, null, null)) { + FilterResult filterResult = FilterRegistry.getFilter().evaluateRequestBody(span, null, null); + if (filterResult.shouldBlock()) { Attribute requestAttr = channel.attr(AttributeKeys.REQUEST); HttpRequest req = ((HttpRequestAndChannel) (requestAttr.get())).request(); - forbidden(ctx, req); + forbidden(ctx, req, filterResult); return; } } ctx.fireChannelRead(msg); } - static void forbidden(ChannelHandlerContext ctx, HttpRequest request) { + static void forbidden(ChannelHandlerContext ctx, HttpRequest request, FilterResult filterResult) { DefaultFullHttpResponse blockResponse = - new DefaultFullHttpResponse(request.getProtocolVersion(), HttpResponseStatus.FORBIDDEN); - blockResponse.headers().add("Content-Length", "0"); + new DefaultFullHttpResponse( + request.getProtocolVersion(), + new HttpResponseStatus( + filterResult.getBlockingStatusCode(), HttpResponseStatus.FORBIDDEN.reasonPhrase()), + Unpooled.copiedBuffer(filterResult.getBlockingMsg().getBytes(StandardCharsets.UTF_8))); + blockResponse + .headers() + .add("Content-Length", String.valueOf(filterResult.getBlockingMsg().length())); ReferenceCountUtil.release(request); ctx.writeAndFlush(blockResponse).addListener(ChannelFutureListener.CLOSE); } diff --git a/instrumentation/netty/netty-4.0/src/test/java/io/opentelemetry/javaagent/instrumentation/hypertrace/netty/v4_0/server/AbstractNetty40ServerInstrumentationTest.java b/instrumentation/netty/netty-4.0/src/test/java/io/opentelemetry/javaagent/instrumentation/hypertrace/netty/v4_0/server/AbstractNetty40ServerInstrumentationTest.java index cd1cd1714..dcc33e401 100644 --- a/instrumentation/netty/netty-4.0/src/test/java/io/opentelemetry/javaagent/instrumentation/hypertrace/netty/v4_0/server/AbstractNetty40ServerInstrumentationTest.java +++ b/instrumentation/netty/netty-4.0/src/test/java/io/opentelemetry/javaagent/instrumentation/hypertrace/netty/v4_0/server/AbstractNetty40ServerInstrumentationTest.java @@ -148,7 +148,7 @@ public void blocking() throws IOException, TimeoutException, InterruptedExceptio try (Response response = httpClient.newCall(request).execute()) { Assertions.assertEquals(403, response.code()); - Assertions.assertTrue(response.body().string().isEmpty()); + Assertions.assertEquals("Hypertrace Blocked Request", response.body().string()); } List> traces = TEST_WRITER.getTraces(); @@ -168,9 +168,7 @@ public void blocking() throws IOException, TimeoutException, InterruptedExceptio .getAttributes() .get(HypertraceSemanticAttributes.httpResponseHeader(RESPONSE_HEADER_NAME))); Assertions.assertNull( - spanData - .getAttributes() - .get(HypertraceSemanticAttributes.httpResponseHeader(RESPONSE_BODY))); + spanData.getAttributes().get(HypertraceSemanticAttributes.HTTP_RESPONSE_BODY)); RequestBody requestBody = blockedRequestBody(true, 3000, 75); Request request2 = @@ -182,7 +180,7 @@ public void blocking() throws IOException, TimeoutException, InterruptedExceptio try (Response response = httpClient.newCall(request2).execute()) { Assertions.assertEquals(403, response.code()); - Assertions.assertTrue(response.body().string().isEmpty()); + Assertions.assertEquals("Hypertrace Blocked Request", response.body().string()); } List> traces2 = TEST_WRITER.getTraces(); @@ -202,9 +200,7 @@ public void blocking() throws IOException, TimeoutException, InterruptedExceptio .getAttributes() .get(HypertraceSemanticAttributes.httpResponseHeader(RESPONSE_HEADER_NAME))); Assertions.assertNull( - spanData2 - .getAttributes() - .get(HypertraceSemanticAttributes.httpResponseHeader(RESPONSE_BODY))); + spanData2.getAttributes().get(HypertraceSemanticAttributes.HTTP_RESPONSE_BODY)); } @Test @@ -264,7 +260,7 @@ public void connectionKeepAlive() throws IOException, TimeoutException, Interrup try (Response response = httpClient.newCall(request2).execute()) { Assertions.assertEquals(403, response.code()); - Assertions.assertTrue(response.body().string().isEmpty()); + Assertions.assertEquals("Hypertrace Blocked Request", response.body().string()); } List> traces2 = TEST_WRITER.getTraces(); diff --git a/instrumentation/netty/netty-4.1/src/main/java/io/opentelemetry/javaagent/instrumentation/hypertrace/netty/v4_1/server/HttpServerBlockingRequestHandler.java b/instrumentation/netty/netty-4.1/src/main/java/io/opentelemetry/javaagent/instrumentation/hypertrace/netty/v4_1/server/HttpServerBlockingRequestHandler.java index 01426c5e7..c4bb452a2 100644 --- a/instrumentation/netty/netty-4.1/src/main/java/io/opentelemetry/javaagent/instrumentation/hypertrace/netty/v4_1/server/HttpServerBlockingRequestHandler.java +++ b/instrumentation/netty/netty-4.1/src/main/java/io/opentelemetry/javaagent/instrumentation/hypertrace/netty/v4_1/server/HttpServerBlockingRequestHandler.java @@ -16,6 +16,7 @@ package io.opentelemetry.javaagent.instrumentation.hypertrace.netty.v4_1.server; +import io.netty.buffer.Unpooled; import io.netty.channel.Channel; import io.netty.channel.ChannelFutureListener; import io.netty.channel.ChannelHandlerContext; @@ -30,7 +31,9 @@ import io.opentelemetry.context.Context; import io.opentelemetry.instrumentation.netty.v4.common.HttpRequestAndChannel; import io.opentelemetry.javaagent.instrumentation.hypertrace.netty.v4_1.AttributeKeys; +import java.nio.charset.StandardCharsets; import java.util.Map; +import org.hypertrace.agent.core.filter.FilterResult; import org.hypertrace.agent.filter.FilterRegistry; public class HttpServerBlockingRequestHandler extends ChannelInboundHandlerAdapter { @@ -51,26 +54,37 @@ public void channelRead(ChannelHandlerContext ctx, Object msg) { if (msg instanceof HttpRequest) { Attribute> headersAttr = channel.attr(AttributeKeys.REQUEST_HEADERS); Map headers = headersAttr.getAndRemove(); - if (headers != null && FilterRegistry.getFilter().evaluateRequestHeaders(span, headers)) { - forbidden(ctx, (HttpRequest) msg); - return; + if (headers != null) { + FilterResult filterResult = + FilterRegistry.getFilter().evaluateRequestHeaders(span, headers); + if (filterResult.shouldBlock()) { + forbidden(ctx, (HttpRequest) msg, filterResult); + return; + } } } if (msg instanceof HttpContent) { - if (FilterRegistry.getFilter().evaluateRequestBody(span, null, null)) { + FilterResult filterResult = FilterRegistry.getFilter().evaluateRequestBody(span, null, null); + if (filterResult.shouldBlock()) { Attribute requestAttr = channel.attr(AttributeKeys.REQUEST); HttpRequest req = ((HttpRequestAndChannel) (requestAttr.get())).request(); - forbidden(ctx, req); + forbidden(ctx, req, filterResult); return; } } ctx.fireChannelRead(msg); } - static void forbidden(ChannelHandlerContext ctx, HttpRequest request) { + static void forbidden(ChannelHandlerContext ctx, HttpRequest request, FilterResult filterResult) { DefaultFullHttpResponse blockResponse = - new DefaultFullHttpResponse(request.protocolVersion(), HttpResponseStatus.FORBIDDEN); - blockResponse.headers().add("Content-Length", "0"); + new DefaultFullHttpResponse( + request.protocolVersion(), + new HttpResponseStatus( + filterResult.getBlockingStatusCode(), HttpResponseStatus.FORBIDDEN.reasonPhrase()), + Unpooled.copiedBuffer(filterResult.getBlockingMsg().getBytes(StandardCharsets.UTF_8))); + blockResponse + .headers() + .add("Content-Length", String.valueOf(filterResult.getBlockingMsg().length())); ReferenceCountUtil.release(request); ctx.writeAndFlush(blockResponse).addListener(ChannelFutureListener.CLOSE); } diff --git a/instrumentation/netty/netty-4.1/src/test/java/io/opentelemetry/javaagent/instrumentation/hypertrace/netty/v4_1/server/AbstractNetty41ServerInstrumentationTest.java b/instrumentation/netty/netty-4.1/src/test/java/io/opentelemetry/javaagent/instrumentation/hypertrace/netty/v4_1/server/AbstractNetty41ServerInstrumentationTest.java index 4bd0a1fad..5ed840db0 100644 --- a/instrumentation/netty/netty-4.1/src/test/java/io/opentelemetry/javaagent/instrumentation/hypertrace/netty/v4_1/server/AbstractNetty41ServerInstrumentationTest.java +++ b/instrumentation/netty/netty-4.1/src/test/java/io/opentelemetry/javaagent/instrumentation/hypertrace/netty/v4_1/server/AbstractNetty41ServerInstrumentationTest.java @@ -148,7 +148,7 @@ public void blocking() throws IOException, TimeoutException, InterruptedExceptio try (Response response = httpClient.newCall(request).execute()) { Assertions.assertEquals(403, response.code()); - Assertions.assertTrue(response.body().string().isEmpty()); + Assertions.assertEquals("Hypertrace Blocked Request", response.body().string()); } List> traces = TEST_WRITER.getTraces(); @@ -168,9 +168,7 @@ public void blocking() throws IOException, TimeoutException, InterruptedExceptio .getAttributes() .get(HypertraceSemanticAttributes.httpResponseHeader(RESPONSE_HEADER_NAME))); Assertions.assertNull( - spanData - .getAttributes() - .get(HypertraceSemanticAttributes.httpResponseHeader(RESPONSE_BODY))); + spanData.getAttributes().get(HypertraceSemanticAttributes.HTTP_RESPONSE_BODY)); RequestBody requestBody = blockedRequestBody(true, 3000, 75); Request request2 = @@ -182,7 +180,7 @@ public void blocking() throws IOException, TimeoutException, InterruptedExceptio try (Response response = httpClient.newCall(request2).execute()) { Assertions.assertEquals(403, response.code()); - Assertions.assertTrue(response.body().string().isEmpty()); + Assertions.assertEquals("Hypertrace Blocked Request", response.body().string()); } List> traces2 = TEST_WRITER.getTraces(); @@ -202,9 +200,7 @@ public void blocking() throws IOException, TimeoutException, InterruptedExceptio .getAttributes() .get(HypertraceSemanticAttributes.httpResponseHeader(RESPONSE_HEADER_NAME))); Assertions.assertNull( - spanData2 - .getAttributes() - .get(HypertraceSemanticAttributes.httpResponseHeader(RESPONSE_BODY))); + spanData2.getAttributes().get(HypertraceSemanticAttributes.HTTP_RESPONSE_BODY)); } @Test @@ -264,7 +260,7 @@ public void connectionKeepAlive() throws IOException, TimeoutException, Interrup try (Response response = httpClient.newCall(request2).execute()) { Assertions.assertEquals(403, response.code()); - Assertions.assertTrue(response.body().string().isEmpty()); + Assertions.assertEquals("Hypertrace Blocked Request", response.body().string()); } List> traces2 = TEST_WRITER.getTraces(); diff --git a/instrumentation/servlet/servlet-3.0/src/main/java/io/opentelemetry/javaagent/instrumentation/hypertrace/servlet/v3_0/nowrapping/Servlet30AndFilterInstrumentation.java b/instrumentation/servlet/servlet-3.0/src/main/java/io/opentelemetry/javaagent/instrumentation/hypertrace/servlet/v3_0/nowrapping/Servlet30AndFilterInstrumentation.java index 371e2581f..526dc0596 100644 --- a/instrumentation/servlet/servlet-3.0/src/main/java/io/opentelemetry/javaagent/instrumentation/hypertrace/servlet/v3_0/nowrapping/Servlet30AndFilterInstrumentation.java +++ b/instrumentation/servlet/servlet-3.0/src/main/java/io/opentelemetry/javaagent/instrumentation/hypertrace/servlet/v3_0/nowrapping/Servlet30AndFilterInstrumentation.java @@ -30,6 +30,7 @@ import io.opentelemetry.javaagent.extension.instrumentation.TypeInstrumentation; import io.opentelemetry.javaagent.extension.instrumentation.TypeTransformer; import java.io.BufferedReader; +import java.io.IOException; import java.io.PrintWriter; import java.util.Collections; import java.util.Enumeration; @@ -45,6 +46,7 @@ import net.bytebuddy.description.type.TypeDescription; import net.bytebuddy.matcher.ElementMatcher; import org.hypertrace.agent.core.config.InstrumentationConfig; +import org.hypertrace.agent.core.filter.FilterResult; import org.hypertrace.agent.core.instrumentation.HypertraceCallDepthThreadLocalMap; import org.hypertrace.agent.core.instrumentation.HypertraceEvaluationException; import org.hypertrace.agent.core.instrumentation.HypertraceSemanticAttributes; @@ -115,8 +117,14 @@ public static boolean start( headers.put(attributeKey.getKey(), headerValue); } - if (FilterRegistry.getFilter().evaluateRequestHeaders(currentSpan, headers)) { - httpResponse.setStatus(403); + FilterResult filterResult = + FilterRegistry.getFilter().evaluateRequestHeaders(currentSpan, headers); + if (filterResult.shouldBlock()) { + try { + httpResponse.getWriter().write(filterResult.getBlockingMsg()); + } catch (IOException ignored) { + } + httpResponse.setStatus(filterResult.getBlockingStatusCode()); // skip execution of the user code return true; } @@ -208,7 +216,12 @@ public static void exit( Throwable tmp = throwable; while (tmp != null) { // loop in case our exception is nested (eg. springframework) if (tmp instanceof HypertraceEvaluationException) { - httpResponse.setStatus(403); + FilterResult filterResult = ((HypertraceEvaluationException) tmp).getFilterResult(); + try { + httpResponse.getWriter().write(filterResult.getBlockingMsg()); + } catch (IOException ignored) { + } + httpResponse.setStatus(filterResult.getBlockingStatusCode()); // bytebuddy treats the reassignment of this variable to null as an instruction to // suppress this exception, which is what we want throwable = null; diff --git a/instrumentation/servlet/servlet-3.0/src/test/java/io/opentelemetry/javaagent/instrumentation/hypertrace/servlet/v3_0/nowrapping/Servlet30InstrumentationTest.java b/instrumentation/servlet/servlet-3.0/src/test/java/io/opentelemetry/javaagent/instrumentation/hypertrace/servlet/v3_0/nowrapping/Servlet30InstrumentationTest.java index d5156ff38..848c390fe 100644 --- a/instrumentation/servlet/servlet-3.0/src/test/java/io/opentelemetry/javaagent/instrumentation/hypertrace/servlet/v3_0/nowrapping/Servlet30InstrumentationTest.java +++ b/instrumentation/servlet/servlet-3.0/src/test/java/io/opentelemetry/javaagent/instrumentation/hypertrace/servlet/v3_0/nowrapping/Servlet30InstrumentationTest.java @@ -278,6 +278,7 @@ public void block() throws Exception { .build(); try (Response response = httpClient.newCall(request).execute()) { Assertions.assertEquals(403, response.code()); + Assertions.assertEquals("Hypertrace Blocked Request", response.body().string()); } TEST_WRITER.waitForTraces(1); @@ -307,6 +308,7 @@ public void blockBody() throws Exception { .build(); try (Response response = httpClient.newCall(request).execute()) { Assertions.assertEquals(403, response.code()); + Assertions.assertEquals("Hypertrace Blocked Request", response.body().string()); } TEST_WRITER.waitForTraces(1); @@ -336,6 +338,7 @@ public void blockBodyWrappedException() throws Exception { .build(); try (Response response = httpClient.newCall(request).execute()) { Assertions.assertEquals(403, response.code()); + Assertions.assertEquals("Hypertrace Blocked Request", response.body().string()); } TEST_WRITER.waitForTraces(1); diff --git a/instrumentation/servlet/servlet-3.0/src/test/java/io/opentelemetry/javaagent/instrumentation/hypertrace/servlet/v3_0/nowrapping/request/ServletInputStreamInstrumentationTest.java b/instrumentation/servlet/servlet-3.0/src/test/java/io/opentelemetry/javaagent/instrumentation/hypertrace/servlet/v3_0/nowrapping/request/ServletInputStreamInstrumentationTest.java index b50900b65..8b4b4b936 100644 --- a/instrumentation/servlet/servlet-3.0/src/test/java/io/opentelemetry/javaagent/instrumentation/hypertrace/servlet/v3_0/nowrapping/request/ServletInputStreamInstrumentationTest.java +++ b/instrumentation/servlet/servlet-3.0/src/test/java/io/opentelemetry/javaagent/instrumentation/hypertrace/servlet/v3_0/nowrapping/request/ServletInputStreamInstrumentationTest.java @@ -26,6 +26,7 @@ import org.ServletStreamContextAccess; import org.TestServletInputStream; import org.hypertrace.agent.core.TriFunction; +import org.hypertrace.agent.core.filter.FilterResult; import org.hypertrace.agent.core.instrumentation.buffer.BoundedBuffersFactory; import org.hypertrace.agent.core.instrumentation.buffer.BoundedByteArrayOutputStream; import org.hypertrace.agent.core.instrumentation.buffer.ByteBufferSpanPair; @@ -91,6 +92,6 @@ public void read_call_depth_read_calls_read() throws IOException { Assertions.assertEquals(BODY.substring(2), buffer.toStringWithSuppliedCharset()); } - private static final TriFunction, Boolean> NOOP_FILTER = - (span, body, headers) -> false; + private static final TriFunction, FilterResult> NOOP_FILTER = + (span, body, headers) -> new FilterResult(false, 0, ""); } diff --git a/instrumentation/servlet/servlet-rw/src/test/java/io/opentelemetry/javaagent/instrumentation/hypertrace/servlet/rw/reader/BufferedReaderInstrumentationTest.java b/instrumentation/servlet/servlet-rw/src/test/java/io/opentelemetry/javaagent/instrumentation/hypertrace/servlet/rw/reader/BufferedReaderInstrumentationTest.java index e2b810b0f..3f65b1cb9 100644 --- a/instrumentation/servlet/servlet-rw/src/test/java/io/opentelemetry/javaagent/instrumentation/hypertrace/servlet/rw/reader/BufferedReaderInstrumentationTest.java +++ b/instrumentation/servlet/servlet-rw/src/test/java/io/opentelemetry/javaagent/instrumentation/hypertrace/servlet/rw/reader/BufferedReaderInstrumentationTest.java @@ -25,6 +25,7 @@ import org.BufferedReaderPrintWriterContextAccess; import org.TestBufferedReader; import org.hypertrace.agent.core.TriFunction; +import org.hypertrace.agent.core.filter.FilterResult; import org.hypertrace.agent.core.instrumentation.buffer.*; import org.hypertrace.agent.testing.AbstractInstrumenterTest; import org.junit.jupiter.api.Assertions; @@ -34,8 +35,8 @@ public class BufferedReaderInstrumentationTest extends AbstractInstrumenterTest private static final String TEST_SPAN_NAME = "foo"; private static final String BODY = "boobar"; - private static final TriFunction, Boolean> NOOP_FILTER = - (span, s, stringStringMap) -> false; + private static final TriFunction, FilterResult> NOOP_FILTER = + (span, s, stringStringMap) -> new FilterResult(false, 0, ""); @Test public void read() throws IOException { diff --git a/instrumentation/spring/spring-webflux-5.0/src/test/java/io/opentelemetry/javaagent/instrumentation/hypertrace/spring/webflux/SpringWebfluxServerTest.java b/instrumentation/spring/spring-webflux-5.0/src/test/java/io/opentelemetry/javaagent/instrumentation/hypertrace/spring/webflux/SpringWebfluxServerTest.java index 9e72548b2..759844ae3 100644 --- a/instrumentation/spring/spring-webflux-5.0/src/test/java/io/opentelemetry/javaagent/instrumentation/hypertrace/spring/webflux/SpringWebfluxServerTest.java +++ b/instrumentation/spring/spring-webflux-5.0/src/test/java/io/opentelemetry/javaagent/instrumentation/hypertrace/spring/webflux/SpringWebfluxServerTest.java @@ -187,7 +187,7 @@ public void blocking() throws IOException, TimeoutException, InterruptedExceptio try (Response response = httpClient.newCall(request).execute()) { Assertions.assertEquals(403, response.code()); - Assertions.assertTrue(response.body().string().isEmpty()); + Assertions.assertEquals("Hypertrace Blocked Request", response.body().string()); } List> traces = TEST_WRITER.getTraces(); diff --git a/instrumentation/vertx/vertx-web-3.0/src/test/java/io/opentelemetry/javaagent/instrumentation/hypertrace/vertx/VertxServerInstrumentationTest.java b/instrumentation/vertx/vertx-web-3.0/src/test/java/io/opentelemetry/javaagent/instrumentation/hypertrace/vertx/VertxServerInstrumentationTest.java index 37e9c6e6e..c2579eba2 100644 --- a/instrumentation/vertx/vertx-web-3.0/src/test/java/io/opentelemetry/javaagent/instrumentation/hypertrace/vertx/VertxServerInstrumentationTest.java +++ b/instrumentation/vertx/vertx-web-3.0/src/test/java/io/opentelemetry/javaagent/instrumentation/hypertrace/vertx/VertxServerInstrumentationTest.java @@ -144,7 +144,7 @@ public void blocking() throws IOException, TimeoutException, InterruptedExceptio try (Response response = httpClient.newCall(request).execute()) { Assertions.assertEquals(403, response.code()); - Assertions.assertTrue(response.body().string().isEmpty()); + Assertions.assertEquals("Hypertrace Blocked Request", response.body().string()); } List> traces = TEST_WRITER.getTraces(); diff --git a/javaagent-core/src/main/java/org/hypertrace/agent/core/filter/FilterResult.java b/javaagent-core/src/main/java/org/hypertrace/agent/core/filter/FilterResult.java new file mode 100644 index 000000000..007fdfb82 --- /dev/null +++ b/javaagent-core/src/main/java/org/hypertrace/agent/core/filter/FilterResult.java @@ -0,0 +1,42 @@ +/* + * Copyright The Hypertrace Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.hypertrace.agent.core.filter; + +public class FilterResult { + + private final boolean shouldBlock; + private final int blockingStatusCode; + private final String blockingMsg; + + public FilterResult(boolean shouldBlock, int blockingStatusCode, String blockingMsg) { + this.shouldBlock = shouldBlock; + this.blockingStatusCode = blockingStatusCode; + this.blockingMsg = blockingMsg; + } + + public boolean shouldBlock() { + return shouldBlock; + } + + public int getBlockingStatusCode() { + return blockingStatusCode; + } + + public String getBlockingMsg() { + return blockingMsg; + } +} diff --git a/javaagent-core/src/main/java/org/hypertrace/agent/core/instrumentation/HypertraceEvaluationException.java b/javaagent-core/src/main/java/org/hypertrace/agent/core/instrumentation/HypertraceEvaluationException.java index 5bba0275a..d6e319414 100644 --- a/javaagent-core/src/main/java/org/hypertrace/agent/core/instrumentation/HypertraceEvaluationException.java +++ b/javaagent-core/src/main/java/org/hypertrace/agent/core/instrumentation/HypertraceEvaluationException.java @@ -16,6 +16,8 @@ package org.hypertrace.agent.core.instrumentation; +import org.hypertrace.agent.core.filter.FilterResult; + /** * A custom exception used to terminate the handling of a request thread after the initial * opportunity for middleware to look at the contents of an HTTP request and determine if it should @@ -34,7 +36,14 @@ public final class HypertraceEvaluationException extends RuntimeException { private static final String DEFAULT_MESSAGE = "A filter implementation determined that this request should be blocked"; - public HypertraceEvaluationException() { + private final FilterResult filterResult; + + public HypertraceEvaluationException(FilterResult filterResult) { super(DEFAULT_MESSAGE); + this.filterResult = filterResult; + } + + public FilterResult getFilterResult() { + return filterResult; } } diff --git a/javaagent-core/src/main/java/org/hypertrace/agent/core/instrumentation/buffer/ByteBufferSpanPair.java b/javaagent-core/src/main/java/org/hypertrace/agent/core/instrumentation/buffer/ByteBufferSpanPair.java index 7c83cce4f..228527837 100644 --- a/javaagent-core/src/main/java/org/hypertrace/agent/core/instrumentation/buffer/ByteBufferSpanPair.java +++ b/javaagent-core/src/main/java/org/hypertrace/agent/core/instrumentation/buffer/ByteBufferSpanPair.java @@ -23,6 +23,7 @@ import java.util.Map; import java.util.Objects; import org.hypertrace.agent.core.TriFunction; +import org.hypertrace.agent.core.filter.FilterResult; import org.hypertrace.agent.core.instrumentation.HypertraceEvaluationException; public class ByteBufferSpanPair { @@ -31,12 +32,12 @@ public class ByteBufferSpanPair { private final BoundedByteArrayOutputStream buffer; private final Map headers; private boolean bufferCaptured; - private final TriFunction, Boolean> filter; + private final TriFunction, FilterResult> filter; public ByteBufferSpanPair( Span span, BoundedByteArrayOutputStream buffer, - TriFunction, Boolean> filter, + TriFunction, FilterResult> filter, Map headers) { this.span = span; this.buffer = buffer; @@ -57,10 +58,10 @@ public void captureBody(AttributeKey attributeKey) { // ignore charset has been parsed before } span.setAttribute(attributeKey, requestBody); - final boolean block; - block = filter.apply(span, requestBody, headers); - if (block) { - throw new HypertraceEvaluationException(); + final FilterResult filterResult; + filterResult = filter.apply(span, requestBody, headers); + if (filterResult.shouldBlock()) { + throw new HypertraceEvaluationException(filterResult); } } diff --git a/javaagent-core/src/main/java/org/hypertrace/agent/core/instrumentation/buffer/CharBufferSpanPair.java b/javaagent-core/src/main/java/org/hypertrace/agent/core/instrumentation/buffer/CharBufferSpanPair.java index 5fc91fda9..34c1c27eb 100644 --- a/javaagent-core/src/main/java/org/hypertrace/agent/core/instrumentation/buffer/CharBufferSpanPair.java +++ b/javaagent-core/src/main/java/org/hypertrace/agent/core/instrumentation/buffer/CharBufferSpanPair.java @@ -21,6 +21,7 @@ import java.io.IOException; import java.util.Map; import org.hypertrace.agent.core.TriFunction; +import org.hypertrace.agent.core.filter.FilterResult; import org.hypertrace.agent.core.instrumentation.HypertraceEvaluationException; public class CharBufferSpanPair { @@ -28,7 +29,7 @@ public class CharBufferSpanPair { public final Span span; public final Map headers; private final BoundedCharArrayWriter buffer; - private final TriFunction, Boolean> filter; + private final TriFunction, FilterResult> filter; /** * A flag to signalize that buffer has been added to span. For instance Jetty calls reader#read in @@ -39,7 +40,7 @@ public class CharBufferSpanPair { public CharBufferSpanPair( Span span, BoundedCharArrayWriter buffer, - TriFunction, Boolean> filter, + TriFunction, FilterResult> filter, Map headers) { this.span = span; this.buffer = buffer; @@ -54,10 +55,10 @@ public void captureBody(AttributeKey attributeKey) { bufferCaptured = true; String requestBody = buffer.toString(); span.setAttribute(attributeKey, requestBody); - final boolean block; - block = filter.apply(span, requestBody, headers); - if (block) { - throw new HypertraceEvaluationException(); + final FilterResult filterResult; + filterResult = filter.apply(span, requestBody, headers); + if (filterResult.shouldBlock()) { + throw new HypertraceEvaluationException(filterResult); } } diff --git a/testing-common/src/testFixtures/java/org/hypertrace/agent/testing/mockfilter/MockFilter.java b/testing-common/src/testFixtures/java/org/hypertrace/agent/testing/mockfilter/MockFilter.java index 6ede7c7f2..626a88942 100644 --- a/testing-common/src/testFixtures/java/org/hypertrace/agent/testing/mockfilter/MockFilter.java +++ b/testing-common/src/testFixtures/java/org/hypertrace/agent/testing/mockfilter/MockFilter.java @@ -20,6 +20,7 @@ import io.opentelemetry.api.trace.Span; import io.opentelemetry.sdk.trace.ReadableSpan; import java.util.Map; +import org.hypertrace.agent.core.filter.FilterResult; import org.hypertrace.agent.filter.api.Filter; /** Mock filter, blocks execution if an attribute with "mockblock" key is present. */ @@ -28,25 +29,26 @@ class MockFilter implements Filter { MockFilter() {} @Override - public boolean evaluateRequestHeaders(Span span, Map headers) { + public FilterResult evaluateRequestHeaders(Span span, Map headers) { if (headers.containsKey("http.request.header.mockblock") || headers.containsKey("rpc.request.metadata.mockblock")) { span.setAttribute("hypertrace.mock.filter.result", "true"); - return true; + return new FilterResult(true, 403, "Hypertrace Blocked Request"); } - return false; + return new FilterResult(false, 403, "Hypertrace Blocked Request"); } @Override - public boolean evaluateRequestBody(Span span, String body, Map headers) { + public FilterResult evaluateRequestBody(Span span, String body, Map headers) { if (body != null && body.contains("block=true")) { - return true; + return new FilterResult(true, 403, "Hypertrace Blocked Request"); } if (span instanceof ReadableSpan) { String spanReqBody = ((ReadableSpan) span).getAttribute(AttributeKey.stringKey("http.request.body")); - return spanReqBody != null && spanReqBody.contains("block=true"); + boolean shouldBlock = spanReqBody != null && spanReqBody.contains("block=true"); + return new FilterResult(shouldBlock, 403, "Hypertrace Blocked Request"); } - return false; + return new FilterResult(false, 403, "Hypertrace Blocked Request"); } }