From bcc9c1d8f4deeba1235f0a275b3d5b7380c8eb45 Mon Sep 17 00:00:00 2001 From: bradAtTraceable Date: Tue, 20 Sep 2022 06:44:26 -0700 Subject: [PATCH 1/2] Build Span Attributes which may be missing from OTEL-provided Span --- .../hypertrace/agent/filter/MultiFilter.java | 41 +++++++++++++++++++ .../hypertrace/agent/filter/api/Filter.java | 21 ++++++++++ .../build.gradle.kts | 1 + instrumentation/build.gradle.kts | 1 + .../netty/netty-4.0/build.gradle.kts | 1 + .../HttpServerBlockingRequestHandler.java | 7 +++- .../netty/netty-4.1/build.gradle.kts | 1 + .../HttpServerBlockingRequestHandler.java | 8 +++- .../servlet/servlet-3.0/build.gradle.kts | 1 + .../Servlet30AndFilterInstrumentation.java | 6 ++- .../servlet/v3_0/nowrapping/Utils.java | 11 +++++ .../v3_0/nowrapping/request/Utils.java | 13 +++++- ...ServletInputStreamInstrumentationTest.java | 15 ++++--- .../BufferedReaderInstrumentationTest.java | 24 +++++++---- .../buffer/ByteBufferSpanPair.java | 14 ++++--- .../buffer/CharBufferSpanPair.java | 15 ++++--- settings.gradle.kts | 2 + 17 files changed, 152 insertions(+), 30 deletions(-) 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..91ce7dbba 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 @@ -51,6 +51,25 @@ public boolean evaluateRequestHeaders(Span span, Map headers) { return shouldBlock; } + @Override + public boolean evaluateRequestHeaders( + Span span, Map headers, Map possiblyMissingSpanAttrs) { + boolean shouldBlock = false; + for (Filter filter : filters) { + try { + if (filter.evaluateRequestHeaders(span, headers, possiblyMissingSpanAttrs)) { + shouldBlock = true; + } + } catch (Throwable t) { + logger.warn( + "Throwable thrown while evaluating Request headers for filter {}", + filter.getClass().getName(), + t); + } + } + return shouldBlock; + } + @Override public boolean evaluateRequestBody(Span span, String body, Map headers) { boolean shouldBlock = false; @@ -68,4 +87,26 @@ public boolean evaluateRequestBody(Span span, String body, Map h } return shouldBlock; } + + @Override + public boolean evaluateRequestBody( + Span span, + String body, + Map headers, + Map possiblyMissingAttrs) { + boolean shouldBlock = false; + for (Filter filter : filters) { + try { + if (filter.evaluateRequestBody(span, body, headers, possiblyMissingAttrs)) { + shouldBlock = true; + } + } catch (Throwable t) { + logger.warn( + "Throwable thrown while evaluating Request body for filter {}", + filter.getClass().getName(), + t); + } + } + return shouldBlock; + } } 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..5dfd46fa2 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 @@ -34,6 +34,19 @@ public interface Filter { */ boolean evaluateRequestHeaders(Span span, Map headers); + /** + * Evaluate the execution (includes extra attributes which might be missing from the span). + * + * @param span + * @param headers + * @param possiblyMissingSpanAttrs + * @return + */ + default boolean evaluateRequestHeaders( + Span span, Map headers, Map possiblyMissingSpanAttrs) { + return evaluateRequestHeaders(span, headers); + } + /** * Evaluate the execution. * @@ -43,4 +56,12 @@ public interface Filter { * @return filter result */ boolean evaluateRequestBody(Span span, String body, Map headers); + + default boolean evaluateRequestBody( + Span span, + String body, + Map headers, + Map possiblyMissingSpanAttrs) { + return evaluateRequestBody(span, body, headers); + } } diff --git a/instrumentation/apache-httpasyncclient-4.1/build.gradle.kts b/instrumentation/apache-httpasyncclient-4.1/build.gradle.kts index 957743c14..daa4d6061 100644 --- a/instrumentation/apache-httpasyncclient-4.1/build.gradle.kts +++ b/instrumentation/apache-httpasyncclient-4.1/build.gradle.kts @@ -35,5 +35,6 @@ dependencies { testImplementation("io.opentelemetry.instrumentation:opentelemetry-instrumentation-api-semconv:${versions["opentelemetry_semconv"]}") library("org.apache.httpcomponents:httpasyncclient:4.1") testImplementation(testFixtures(project(":testing-common"))) + implementation(project(":instrumentation:utils")) } diff --git a/instrumentation/build.gradle.kts b/instrumentation/build.gradle.kts index 065d5736b..be8a4849f 100644 --- a/instrumentation/build.gradle.kts +++ b/instrumentation/build.gradle.kts @@ -19,6 +19,7 @@ dependencies{ implementation(project(":instrumentation:undertow:undertow-1.4")) implementation(project(":instrumentation:undertow:undertow-servlet-1.4")) implementation(project(":instrumentation:vertx:vertx-web-3.0")) + implementation(project(":instrumentation:utils")) implementation(project(":otel-extensions")) } diff --git a/instrumentation/netty/netty-4.0/build.gradle.kts b/instrumentation/netty/netty-4.0/build.gradle.kts index 8102439ed..90e8e78f8 100644 --- a/instrumentation/netty/netty-4.0/build.gradle.kts +++ b/instrumentation/netty/netty-4.0/build.gradle.kts @@ -88,5 +88,6 @@ dependencies { testImplementation(testFixtures(project(":testing-common"))) testImplementation("org.asynchttpclient:async-http-client:2.0.9") + implementation(project(":instrumentation:utils")) } 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 ce5987de7..e784dc34d 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 @@ -28,6 +28,7 @@ import io.opentelemetry.api.trace.Span; import io.opentelemetry.context.Context; import io.opentelemetry.javaagent.instrumentation.hypertrace.netty.v4_0.AttributeKeys; +import io.opentelemetry.javaagent.instrumentation.hypertrace.utils.SpanUtils; import java.util.Map; import org.hypertrace.agent.filter.FilterRegistry; @@ -50,7 +51,11 @@ 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)) { + Map possiblyMissingAttrs = + SpanUtils.getPossiblyMissingSpanAttributesFromURI(((HttpRequest) msg).getUri()); + if (headers != null + && FilterRegistry.getFilter() + .evaluateRequestHeaders(span, headers, possiblyMissingAttrs)) { forbidden(ctx, (HttpRequest) msg); return; } diff --git a/instrumentation/netty/netty-4.1/build.gradle.kts b/instrumentation/netty/netty-4.1/build.gradle.kts index 88c503893..7d42b5546 100644 --- a/instrumentation/netty/netty-4.1/build.gradle.kts +++ b/instrumentation/netty/netty-4.1/build.gradle.kts @@ -53,5 +53,6 @@ dependencies { testImplementation(testFixtures(project(":testing-common"))) testImplementation("io.netty:netty-handler:4.1.0.Final") testImplementation("org.asynchttpclient:async-http-client:2.1.0") + implementation(project(":instrumentation:utils")) } 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 bf60b1934..924095383 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 @@ -28,6 +28,7 @@ import io.opentelemetry.api.trace.Span; import io.opentelemetry.context.Context; import io.opentelemetry.javaagent.instrumentation.hypertrace.netty.v4_1.AttributeKeys; +import io.opentelemetry.javaagent.instrumentation.hypertrace.utils.SpanUtils; import java.util.Map; import org.hypertrace.agent.filter.FilterRegistry; @@ -50,7 +51,12 @@ 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)) { + Map possiblyMissingAttrs = + SpanUtils.getPossiblyMissingSpanAttributesFromURI(((HttpRequest) msg).uri()); + + if (headers != null + && FilterRegistry.getFilter() + .evaluateRequestHeaders(span, headers, possiblyMissingAttrs)) { forbidden(ctx, (HttpRequest) msg); return; } diff --git a/instrumentation/servlet/servlet-3.0/build.gradle.kts b/instrumentation/servlet/servlet-3.0/build.gradle.kts index 0720a6445..9ee50512e 100644 --- a/instrumentation/servlet/servlet-3.0/build.gradle.kts +++ b/instrumentation/servlet/servlet-3.0/build.gradle.kts @@ -46,4 +46,5 @@ dependencies { testImplementation("org.eclipse.jetty:jetty-server:8.1.22.v20160922") testImplementation("org.eclipse.jetty:jetty-servlet:8.1.22.v20160922") testImplementation("io.opentelemetry.instrumentation:opentelemetry-instrumentation-api-semconv:${versions["opentelemetry_semconv"]}") + implementation(project(":instrumentation:utils")) } 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 74a2aacb3..374a28f0a 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 @@ -115,7 +115,11 @@ public static boolean start( headers.put(attributeKey.getKey(), headerValue); } - if (FilterRegistry.getFilter().evaluateRequestHeaders(currentSpan, headers)) { + Map possiblyMissingAttrs = + Utils.getPossiblyMissingSpanAttributes(httpRequest); + + if (FilterRegistry.getFilter() + .evaluateRequestHeaders(currentSpan, headers, possiblyMissingAttrs)) { httpResponse.setStatus(403); // skip execution of the user code return true; diff --git a/instrumentation/servlet/servlet-3.0/src/main/java/io/opentelemetry/javaagent/instrumentation/hypertrace/servlet/v3_0/nowrapping/Utils.java b/instrumentation/servlet/servlet-3.0/src/main/java/io/opentelemetry/javaagent/instrumentation/hypertrace/servlet/v3_0/nowrapping/Utils.java index 3c271ca11..ce259cb5f 100644 --- a/instrumentation/servlet/servlet-3.0/src/main/java/io/opentelemetry/javaagent/instrumentation/hypertrace/servlet/v3_0/nowrapping/Utils.java +++ b/instrumentation/servlet/servlet-3.0/src/main/java/io/opentelemetry/javaagent/instrumentation/hypertrace/servlet/v3_0/nowrapping/Utils.java @@ -18,6 +18,7 @@ import io.opentelemetry.api.trace.Span; import io.opentelemetry.instrumentation.api.field.VirtualField; +import io.opentelemetry.javaagent.instrumentation.hypertrace.utils.SpanUtils; import java.io.BufferedReader; import java.io.PrintWriter; import java.io.UnsupportedEncodingException; @@ -121,4 +122,14 @@ public static void resetRequestBodyBuffers( } } } + + /** + * Creates a Map of those attributes that may be missing from the Span. + * + * @param request + * @return + */ + public static Map getPossiblyMissingSpanAttributes(HttpServletRequest request) { + return SpanUtils.getPossiblyMissingSpanAttributes(request); + } } diff --git a/instrumentation/servlet/servlet-3.0/src/main/java/io/opentelemetry/javaagent/instrumentation/hypertrace/servlet/v3_0/nowrapping/request/Utils.java b/instrumentation/servlet/servlet-3.0/src/main/java/io/opentelemetry/javaagent/instrumentation/hypertrace/servlet/v3_0/nowrapping/request/Utils.java index 27d412c9b..3ddc1156e 100644 --- a/instrumentation/servlet/servlet-3.0/src/main/java/io/opentelemetry/javaagent/instrumentation/hypertrace/servlet/v3_0/nowrapping/request/Utils.java +++ b/instrumentation/servlet/servlet-3.0/src/main/java/io/opentelemetry/javaagent/instrumentation/hypertrace/servlet/v3_0/nowrapping/request/Utils.java @@ -43,11 +43,16 @@ public static ByteBufferSpanPair createRequestByteBufferSpanPair( if (contentLength < 0) { contentLength = ContentLengthUtils.DEFAULT; } + Map possiblyMissingAttrs = + io.opentelemetry.javaagent.instrumentation.hypertrace.servlet.v3_0.nowrapping.Utils + .getPossiblyMissingSpanAttributes(httpServletRequest); + return new ByteBufferSpanPair( span, BoundedBuffersFactory.createStream(contentLength, charset), filter::evaluateRequestBody, - headers); + headers, + possiblyMissingAttrs); } public static CharBufferSpanPair createRequestCharBufferSpanPair( @@ -56,11 +61,15 @@ public static CharBufferSpanPair createRequestCharBufferSpanPair( if (contentLength < 0) { contentLength = ContentLengthUtils.DEFAULT; } + Map possiblyMissingAttrs = + io.opentelemetry.javaagent.instrumentation.hypertrace.servlet.v3_0.nowrapping.Utils + .getPossiblyMissingSpanAttributes(httpServletRequest); return new CharBufferSpanPair( span, BoundedBuffersFactory.createWriter(contentLength), filter::evaluateRequestBody, - headers); + headers, + possiblyMissingAttrs); } /** 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..fc04b316f 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 @@ -25,7 +25,7 @@ import javax.servlet.ServletInputStream; import org.ServletStreamContextAccess; import org.TestServletInputStream; -import org.hypertrace.agent.core.TriFunction; +import org.hypertrace.agent.core.QuadFunction; import org.hypertrace.agent.core.instrumentation.buffer.BoundedBuffersFactory; import org.hypertrace.agent.core.instrumentation.buffer.BoundedByteArrayOutputStream; import org.hypertrace.agent.core.instrumentation.buffer.ByteBufferSpanPair; @@ -48,7 +48,8 @@ public void read() throws IOException { BoundedByteArrayOutputStream buffer = BoundedBuffersFactory.createStream(StandardCharsets.UTF_8); ByteBufferSpanPair bufferSpanPair = - new ByteBufferSpanPair(span, buffer, NOOP_FILTER, Collections.emptyMap()); + new ByteBufferSpanPair( + span, buffer, NOOP_FILTER, Collections.emptyMap(), Collections.emptyMap()); ServletStreamContextAccess.addToInputStreamContext(servletInputStream, bufferSpanPair); while (servletInputStream.read() != -1) {} @@ -66,7 +67,8 @@ public void read_callDepth_is_cleared() throws IOException { BoundedByteArrayOutputStream buffer = BoundedBuffersFactory.createStream(StandardCharsets.UTF_8); ByteBufferSpanPair bufferSpanPair = - new ByteBufferSpanPair(span, buffer, NOOP_FILTER, Collections.emptyMap()); + new ByteBufferSpanPair( + span, buffer, NOOP_FILTER, Collections.emptyMap(), Collections.emptyMap()); ServletStreamContextAccess.addToInputStreamContext(servletInputStream, bufferSpanPair); while (servletInputStream.read() != -1) {} @@ -84,13 +86,14 @@ public void read_call_depth_read_calls_read() throws IOException { BoundedByteArrayOutputStream buffer = BoundedBuffersFactory.createStream(StandardCharsets.UTF_8); ByteBufferSpanPair bufferSpanPair = - new ByteBufferSpanPair(span, buffer, NOOP_FILTER, Collections.emptyMap()); + new ByteBufferSpanPair( + span, buffer, NOOP_FILTER, Collections.emptyMap(), Collections.emptyMap()); ServletStreamContextAccess.addToInputStreamContext(servletInputStream, bufferSpanPair); servletInputStream.read(new byte[BODY.length()]); Assertions.assertEquals(BODY.substring(2), buffer.toStringWithSuppliedCharset()); } - private static final TriFunction, Boolean> NOOP_FILTER = - (span, body, headers) -> false; + private static final QuadFunction, Map, Boolean> + NOOP_FILTER = (span, body, headers, missingAttrs) -> false; } 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..9e16b707b 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 @@ -24,7 +24,7 @@ import java.util.Map; import org.BufferedReaderPrintWriterContextAccess; import org.TestBufferedReader; -import org.hypertrace.agent.core.TriFunction; +import org.hypertrace.agent.core.QuadFunction; import org.hypertrace.agent.core.instrumentation.buffer.*; import org.hypertrace.agent.testing.AbstractInstrumenterTest; import org.junit.jupiter.api.Assertions; @@ -34,8 +34,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 QuadFunction, Map, Boolean> + NOOP_FILTER = (span, s, stringStringMap, missingAttrMap) -> false; @Test public void read() throws IOException { @@ -45,7 +45,8 @@ public void read() throws IOException { BoundedCharArrayWriter buffer = BoundedBuffersFactory.createWriter(); CharBufferSpanPair bufferSpanPair = - new CharBufferSpanPair(span, buffer, NOOP_FILTER, Collections.emptyMap()); + new CharBufferSpanPair( + span, buffer, NOOP_FILTER, Collections.emptyMap(), Collections.emptyMap()); BufferedReaderPrintWriterContextAccess.addToBufferedReaderContext( bufferedReader, bufferSpanPair); @@ -63,7 +64,8 @@ public void read_callDepth_isCleared() throws IOException { BoundedCharArrayWriter buffer = BoundedBuffersFactory.createWriter(); CharBufferSpanPair bufferSpanPair = - new CharBufferSpanPair(span, buffer, NOOP_FILTER, Collections.emptyMap()); + new CharBufferSpanPair( + span, buffer, NOOP_FILTER, Collections.emptyMap(), Collections.emptyMap()); BufferedReaderPrintWriterContextAccess.addToBufferedReaderContext( bufferedReader, bufferSpanPair); @@ -80,7 +82,8 @@ public void read_char_arr() throws IOException { BoundedCharArrayWriter buffer = BoundedBuffersFactory.createWriter(); CharBufferSpanPair bufferSpanPair = - new CharBufferSpanPair(span, buffer, NOOP_FILTER, Collections.emptyMap()); + new CharBufferSpanPair( + span, buffer, NOOP_FILTER, Collections.emptyMap(), Collections.emptyMap()); BufferedReaderPrintWriterContextAccess.addToBufferedReaderContext( bufferedReader, bufferSpanPair); @@ -98,7 +101,8 @@ public void read_callDepth_char_arr() throws IOException { BoundedCharArrayWriter buffer = BoundedBuffersFactory.createWriter(); CharBufferSpanPair bufferSpanPair = - new CharBufferSpanPair(span, buffer, NOOP_FILTER, Collections.emptyMap()); + new CharBufferSpanPair( + span, buffer, NOOP_FILTER, Collections.emptyMap(), Collections.emptyMap()); BufferedReaderPrintWriterContextAccess.addToBufferedReaderContext( bufferedReader, bufferSpanPair); @@ -115,7 +119,8 @@ public void read_char_arr_offset() throws IOException { BoundedCharArrayWriter buffer = BoundedBuffersFactory.createWriter(); CharBufferSpanPair bufferSpanPair = - new CharBufferSpanPair(span, buffer, NOOP_FILTER, Collections.emptyMap()); + new CharBufferSpanPair( + span, buffer, NOOP_FILTER, Collections.emptyMap(), Collections.emptyMap()); BufferedReaderPrintWriterContextAccess.addToBufferedReaderContext( bufferedReader, bufferSpanPair); @@ -134,7 +139,8 @@ public void readLine() throws IOException { BoundedCharArrayWriter buffer = BoundedBuffersFactory.createWriter(); CharBufferSpanPair bufferSpanPair = - new CharBufferSpanPair(span, buffer, NOOP_FILTER, Collections.emptyMap()); + new CharBufferSpanPair( + span, buffer, NOOP_FILTER, Collections.emptyMap(), Collections.emptyMap()); BufferedReaderPrintWriterContextAccess.addToBufferedReaderContext( bufferedReader, bufferSpanPair); 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..427f1af7b 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 @@ -22,7 +22,7 @@ import java.io.UnsupportedEncodingException; import java.util.Map; import java.util.Objects; -import org.hypertrace.agent.core.TriFunction; +import org.hypertrace.agent.core.QuadFunction; import org.hypertrace.agent.core.instrumentation.HypertraceEvaluationException; public class ByteBufferSpanPair { @@ -31,17 +31,21 @@ public class ByteBufferSpanPair { private final BoundedByteArrayOutputStream buffer; private final Map headers; private boolean bufferCaptured; - private final TriFunction, Boolean> filter; + private Map possiblyMissingAttrs; + private final QuadFunction, Map, Boolean> + filter; public ByteBufferSpanPair( Span span, BoundedByteArrayOutputStream buffer, - TriFunction, Boolean> filter, - Map headers) { + QuadFunction, Map, Boolean> filter, + Map headers, + Map possiblyMissingAttrs) { this.span = span; this.buffer = buffer; this.filter = Objects.requireNonNull(filter); this.headers = headers; + this.possiblyMissingAttrs = possiblyMissingAttrs; } public void captureBody(AttributeKey attributeKey) { @@ -58,7 +62,7 @@ public void captureBody(AttributeKey attributeKey) { } span.setAttribute(attributeKey, requestBody); final boolean block; - block = filter.apply(span, requestBody, headers); + block = filter.apply(span, requestBody, headers, possiblyMissingAttrs); if (block) { throw new HypertraceEvaluationException(); } 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..fa40d1624 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 @@ -20,7 +20,7 @@ import io.opentelemetry.api.trace.Span; import java.io.IOException; import java.util.Map; -import org.hypertrace.agent.core.TriFunction; +import org.hypertrace.agent.core.QuadFunction; import org.hypertrace.agent.core.instrumentation.HypertraceEvaluationException; public class CharBufferSpanPair { @@ -28,7 +28,8 @@ public class CharBufferSpanPair { public final Span span; public final Map headers; private final BoundedCharArrayWriter buffer; - private final TriFunction, Boolean> filter; + private final QuadFunction, Map, Boolean> + filter; /** * A flag to signalize that buffer has been added to span. For instance Jetty calls reader#read in @@ -36,15 +37,19 @@ public class CharBufferSpanPair { */ private boolean bufferCaptured; + private Map possiblyMissingAttrs; + public CharBufferSpanPair( Span span, BoundedCharArrayWriter buffer, - TriFunction, Boolean> filter, - Map headers) { + QuadFunction, Map, Boolean> filter, + Map headers, + Map possiblyMissingAttrs) { this.span = span; this.buffer = buffer; this.headers = headers; this.filter = filter; + this.possiblyMissingAttrs = possiblyMissingAttrs; } public void captureBody(AttributeKey attributeKey) { @@ -55,7 +60,7 @@ public void captureBody(AttributeKey attributeKey) { String requestBody = buffer.toString(); span.setAttribute(attributeKey, requestBody); final boolean block; - block = filter.apply(span, requestBody, headers); + block = filter.apply(span, requestBody, headers, possiblyMissingAttrs); if (block) { throw new HypertraceEvaluationException(); } diff --git a/settings.gradle.kts b/settings.gradle.kts index 2edeb428b..4484ff1d2 100644 --- a/settings.gradle.kts +++ b/settings.gradle.kts @@ -66,3 +66,5 @@ include("instrumentation:undertow:undertow-1.4") findProject(":instrumentation:undertow:undertow-1.4")?.name = "undertow-1.4" include("instrumentation:undertow:undertow-servlet-1.4") findProject(":instrumentation:undertow:undertow-servlet-1.4")?.name = "undertow-servlet-1.4" +include("instrumentation:utils") +findProject(":instrumentation:utils")?.name = "utils" From 043ac8f0d244c23f00b680071d709c903ca63b80 Mon Sep 17 00:00:00 2001 From: bradAtTraceable Date: Tue, 20 Sep 2022 07:24:45 -0700 Subject: [PATCH 2/2] Forgot to check in new files --- instrumentation/utils/build.gradle.kts | 13 + .../hypertrace/utils/SpanUtils.java | 142 +++++++ .../hypertrace/utils/SpanUtilsTest.java | 399 ++++++++++++++++++ .../hypertrace/agent/core/QuadFunction.java | 22 + 4 files changed, 576 insertions(+) create mode 100644 instrumentation/utils/build.gradle.kts create mode 100644 instrumentation/utils/src/main/java/io/opentelemetry/javaagent/instrumentation/hypertrace/utils/SpanUtils.java create mode 100644 instrumentation/utils/src/test/java/io/opentelemetry/javaagent/instrumentation/hypertrace/utils/SpanUtilsTest.java create mode 100644 javaagent-core/src/main/java/org/hypertrace/agent/core/QuadFunction.java diff --git a/instrumentation/utils/build.gradle.kts b/instrumentation/utils/build.gradle.kts new file mode 100644 index 000000000..1c379543d --- /dev/null +++ b/instrumentation/utils/build.gradle.kts @@ -0,0 +1,13 @@ +plugins { + `java-library` + id("net.bytebuddy.byte-buddy") + id("io.opentelemetry.instrumentation.auto-instrumentation") + muzzle +} + +val versions: Map by extra + +dependencies{ + implementation("javax.servlet:javax.servlet-api:3.1.0") + implementation("org.slf4j:slf4j-api:${versions["slf4j"]}") +} diff --git a/instrumentation/utils/src/main/java/io/opentelemetry/javaagent/instrumentation/hypertrace/utils/SpanUtils.java b/instrumentation/utils/src/main/java/io/opentelemetry/javaagent/instrumentation/hypertrace/utils/SpanUtils.java new file mode 100644 index 000000000..bc7928fa1 --- /dev/null +++ b/instrumentation/utils/src/main/java/io/opentelemetry/javaagent/instrumentation/hypertrace/utils/SpanUtils.java @@ -0,0 +1,142 @@ +/* + * 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 io.opentelemetry.javaagent.instrumentation.hypertrace.utils; + +import java.net.URI; +import java.net.URL; +import java.util.ArrayList; +import java.util.HashMap; +import java.util.List; +import java.util.Map; +import java.util.function.BiConsumer; +import javax.servlet.http.HttpServletRequest; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +/** Contains static utility methods for Spans. */ +public abstract class SpanUtils { + + static final String HTTP_URL_ATTRIBUTE_KEY = "http.url"; + private static final Logger logger = LoggerFactory.getLogger(SpanUtils.class); + + /** + * A List of BiConsumer implementations, each of which takes an HttpServletRequest and creates an + * attribute that may be missing from the Span. + */ + private static final List>> + MISSING_ATTRIBUTE_PROVIDERS_WITH_REQUEST = initMissingAttributeProvidersWithRequest(); + + /** + * A List of BiConsumer implementations, each of which takes a URI and creates an attribute that + * may be missing from the Span. + */ + private static final List>> + MISSING_ATTRIBUTE_PROVIDERS_WITH_URI = initMissingAttributeProvidersWithURI(); + + private static List>> + initMissingAttributeProvidersWithRequest() { + List>> returnList = new ArrayList<>(); + + returnList.add(SpanUtils::addHttpURLAttributeToSpanFromRequest); + return returnList; + } + + private static List>> + initMissingAttributeProvidersWithURI() { + List>> returnList = new ArrayList<>(); + + returnList.add(SpanUtils::addHttpURLAttributeToSpanFromURI); + return returnList; + } + + /** + * Returns a Map of attributes, each of which represents an attribute that may be missing from the + * Span. + * + *

Note that the Span itself is not an argument: the Span attributes cannot be accessed to + * determine if the attributes being built here are required. They are built unconditionally; they + * must be compared to the Span attributes at a later stage, when the existing Span attributes can + * be accessed. + * + * @param servletRequest The HttpServletRequest, which is used to build the (possibly) missing + * attributes. + * @return + */ + public static Map getPossiblyMissingSpanAttributes( + HttpServletRequest servletRequest) { + Map returnMap = new HashMap<>(); + + for (BiConsumer> nextMissingAttrConsumer : + MISSING_ATTRIBUTE_PROVIDERS_WITH_REQUEST) { + nextMissingAttrConsumer.accept(servletRequest, returnMap); + } + + return returnMap; + } + + /** + * Returns a Map of attributes, each of which represents an attribute that may be missing from the + * Span. + * + *

Note that the Span itself is not an argument: the Span attributes cannot be accessed to + * determine if the attributes being built here are required. They are built unconditionally; they + * must be compared to the Span attributes at a later stage, when the existing Span attributes can + * be accessed. + * + * @param uri A String containing the URI, which is used to build the (possibly) missing + * attributes. + * @return + */ + public static Map getPossiblyMissingSpanAttributesFromURI(String uri) { + Map returnMap = new HashMap<>(); + for (BiConsumer> nextMissingAttrConsumer : + MISSING_ATTRIBUTE_PROVIDERS_WITH_URI) { + nextMissingAttrConsumer.accept(uri, returnMap); + } + + return returnMap; + } + + /** + * Builds an "http.url" attribute from the HttpServletRequest + * + * @param servletRequest + * @param newAttrMap + */ + private static void addHttpURLAttributeToSpanFromRequest( + HttpServletRequest servletRequest, Map newAttrMap) { + + newAttrMap.put(HTTP_URL_ATTRIBUTE_KEY, servletRequest.getRequestURL().toString()); + } + + /** + * Builds an "http.url" attribute from the URI + * + * @param uriAsString + * @param newAttrMap + */ + private static void addHttpURLAttributeToSpanFromURI( + String uriAsString, Map newAttrMap) { + try { + URI uri = new URI(uriAsString); + URL url = uri.toURL(); + newAttrMap.put(HTTP_URL_ATTRIBUTE_KEY, url.toString()); + } catch (Exception e) { + logger.error("Unable to add URL to Span: {}", e.toString()); + } + } +} diff --git a/instrumentation/utils/src/test/java/io/opentelemetry/javaagent/instrumentation/hypertrace/utils/SpanUtilsTest.java b/instrumentation/utils/src/test/java/io/opentelemetry/javaagent/instrumentation/hypertrace/utils/SpanUtilsTest.java new file mode 100644 index 000000000..d72ab45da --- /dev/null +++ b/instrumentation/utils/src/test/java/io/opentelemetry/javaagent/instrumentation/hypertrace/utils/SpanUtilsTest.java @@ -0,0 +1,399 @@ +/* + * 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 io.opentelemetry.javaagent.instrumentation.hypertrace.utils; + +import java.io.BufferedReader; +import java.io.IOException; +import java.io.UnsupportedEncodingException; +import java.net.MalformedURLException; +import java.net.URL; +import java.security.Principal; +import java.util.Collection; +import java.util.Enumeration; +import java.util.Locale; +import java.util.Map; +import javax.servlet.*; +import javax.servlet.http.*; +import org.junit.jupiter.api.Assertions; +import org.junit.jupiter.api.Test; + +public class SpanUtilsTest { + + private static final String REQUEST_URL_AS_STRING = "http://myServer:8080/appName"; + + private static class MockHttpServletRequest implements HttpServletRequest { + + private final URL url; + + public MockHttpServletRequest(String urlText) throws MalformedURLException { + super(); + + url = new URL(urlText); + } + + @Override + public String getAuthType() { + return null; + } + + @Override + public Cookie[] getCookies() { + return new Cookie[0]; + } + + @Override + public long getDateHeader(String name) { + return 0; + } + + @Override + public String getHeader(String name) { + return null; + } + + @Override + public Enumeration getHeaders(String name) { + return null; + } + + @Override + public Enumeration getHeaderNames() { + return null; + } + + @Override + public int getIntHeader(String name) { + return 0; + } + + @Override + public String getMethod() { + return null; + } + + @Override + public String getPathInfo() { + return null; + } + + @Override + public String getPathTranslated() { + return null; + } + + @Override + public String getContextPath() { + return null; + } + + @Override + public String getQueryString() { + return null; + } + + @Override + public String getRemoteUser() { + return null; + } + + @Override + public boolean isUserInRole(String role) { + return false; + } + + @Override + public Principal getUserPrincipal() { + return null; + } + + @Override + public String getRequestedSessionId() { + return null; + } + + @Override + public String getRequestURI() { + return null; + } + + @Override + public StringBuffer getRequestURL() { + return new StringBuffer(url.toString()); + } + + @Override + public String getServletPath() { + return null; + } + + @Override + public HttpSession getSession(boolean create) { + return null; + } + + @Override + public HttpSession getSession() { + return null; + } + + @Override + public String changeSessionId() { + return null; + } + + @Override + public boolean isRequestedSessionIdValid() { + return false; + } + + @Override + public boolean isRequestedSessionIdFromCookie() { + return false; + } + + @Override + public boolean isRequestedSessionIdFromURL() { + return false; + } + + @Override + public boolean isRequestedSessionIdFromUrl() { + return false; + } + + @Override + public boolean authenticate(HttpServletResponse response) throws IOException, ServletException { + return false; + } + + @Override + public void login(String username, String password) throws ServletException {} + + @Override + public void logout() throws ServletException {} + + @Override + public Collection getParts() throws IOException, ServletException { + return null; + } + + @Override + public Part getPart(String name) throws IOException, ServletException { + return null; + } + + @Override + public T upgrade(Class handlerClass) + throws IOException, ServletException { + return null; + } + + @Override + public Object getAttribute(String name) { + return null; + } + + @Override + public Enumeration getAttributeNames() { + return null; + } + + @Override + public String getCharacterEncoding() { + return null; + } + + @Override + public void setCharacterEncoding(String env) throws UnsupportedEncodingException {} + + @Override + public int getContentLength() { + return 0; + } + + @Override + public long getContentLengthLong() { + return 0; + } + + @Override + public String getContentType() { + return null; + } + + @Override + public ServletInputStream getInputStream() throws IOException { + return null; + } + + @Override + public String getParameter(String name) { + return null; + } + + @Override + public Enumeration getParameterNames() { + return null; + } + + @Override + public String[] getParameterValues(String name) { + return new String[0]; + } + + @Override + public Map getParameterMap() { + return null; + } + + @Override + public String getProtocol() { + return null; + } + + @Override + public String getScheme() { + return null; + } + + @Override + public String getServerName() { + return null; + } + + @Override + public int getServerPort() { + return 0; + } + + @Override + public BufferedReader getReader() throws IOException { + return null; + } + + @Override + public String getRemoteAddr() { + return null; + } + + @Override + public String getRemoteHost() { + return null; + } + + @Override + public void setAttribute(String name, Object o) {} + + @Override + public void removeAttribute(String name) {} + + @Override + public Locale getLocale() { + return null; + } + + @Override + public Enumeration getLocales() { + return null; + } + + @Override + public boolean isSecure() { + return false; + } + + @Override + public RequestDispatcher getRequestDispatcher(String path) { + return null; + } + + @Override + public String getRealPath(String path) { + return null; + } + + @Override + public int getRemotePort() { + return 0; + } + + @Override + public String getLocalName() { + return null; + } + + @Override + public String getLocalAddr() { + return null; + } + + @Override + public int getLocalPort() { + return 0; + } + + @Override + public ServletContext getServletContext() { + return null; + } + + @Override + public AsyncContext startAsync() throws IllegalStateException { + return null; + } + + @Override + public AsyncContext startAsync(ServletRequest servletRequest, ServletResponse servletResponse) + throws IllegalStateException { + return null; + } + + @Override + public boolean isAsyncStarted() { + return false; + } + + @Override + public boolean isAsyncSupported() { + return false; + } + + @Override + public AsyncContext getAsyncContext() { + return null; + } + + @Override + public DispatcherType getDispatcherType() { + return null; + } + } + + @Test + public void testGetPossiblyMissingSpanAttributes() throws MalformedURLException { + HttpServletRequest servletRequest = new MockHttpServletRequest(REQUEST_URL_AS_STRING); + + Map possiblyMissingSpanAttributes = + SpanUtils.getPossiblyMissingSpanAttributes(servletRequest); + + String value = possiblyMissingSpanAttributes.get(SpanUtils.HTTP_URL_ATTRIBUTE_KEY); + + Assertions.assertNotNull(value); + + Assertions.assertEquals(REQUEST_URL_AS_STRING, value); + } +} diff --git a/javaagent-core/src/main/java/org/hypertrace/agent/core/QuadFunction.java b/javaagent-core/src/main/java/org/hypertrace/agent/core/QuadFunction.java new file mode 100644 index 000000000..7c538e919 --- /dev/null +++ b/javaagent-core/src/main/java/org/hypertrace/agent/core/QuadFunction.java @@ -0,0 +1,22 @@ +/* + * 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; + +@FunctionalInterface +public interface QuadFunction { + R apply(P1 p1, P2 p2, P3 p3, P4 p4); +}